diff --git a/.gitignore b/.gitignore index 78a44107..44366299 100644 --- a/.gitignore +++ b/.gitignore @@ -573,6 +573,12 @@ coverage.xml # retrieval, read-once-per-file. Rebuilds automatically on content change. **/.cache/doc-index/ +# OKF bundle cache (okf.py) — local, regenerable concept files + index for +# JIT retrieval's semantic fallback. Never committed: a fresh clone rebuilds +# it (at real token cost, via an external LLM caller) rather than relying on +# a stale copy shipped in version control. +**/.cache/okf/ + # Superpowers brainstorming/planning specs (local-only) docs/superpowers/ diff --git a/architecture/features/core-infra.md b/architecture/features/core-infra.md index 47b95e34..3d14f324 100644 --- a/architecture/features/core-infra.md +++ b/architecture/features/core-infra.md @@ -596,6 +596,12 @@ Enables users to install Studio globally, initialize it in any project with sens - [x] - `p1` - `file_action`: file-change icon printer (created/updated/unchanged/etc.) to stderr - `inst-ui-file-action` - [x] - `p1` - `result` JSON branch: serialize result dict as JSON to stdout in `--json` mode - `inst-ui-result-json` - [x] - `p1` - `result` human branch: invoke `human_fn` or generic status/message fallback to stderr - `inst-ui-result-human` +- [x] - `p1` - `require_existing_file`: resolve a CLI file-path argument, emitting the standard "File not found" ERROR result and returning `None` when it doesn't exist -- shared by every single-file-argument command - `inst-ui-require-existing-file` +- [x] - `p1` - `JsonSafeArgumentParser`/`parse_args_or_json_error`: an `ArgumentParser` whose parsing failures raise instead of printing a plain-text usage banner and exiting directly, so a missing/malformed argument still honors the `--json` output contract (`--help`/`--version` are unaffected, since those exit via a different path) - `inst-ui-json-safe-argparse` +- [x] - `p1` - `parse_file_command`: the combined "parse args safely, then require an existing file" two-step every single-file-argument command needs, extracted once a third command repeated the pattern identically enough for pylint's duplicate-code check to catch it - `inst-ui-parse-file-command` +- [x] - `p1` - `report_read_error`: the standard "Cannot read file" ERROR result for an `OSError`/`UnicodeDecodeError` raised while reading a file `parse_file_command` already confirmed exists, extracted once a second command repeated the identical try/except/result block - `inst-ui-report-read-error` +- [x] - `p1` - `call_with_read_error_handling`: call a zero-arg callable that reads a file, catching `OSError`/`UnicodeDecodeError` and reporting via `report_read_error`, extracted once the identical try/except block appeared across four commands - `inst-ui-call-with-read-error-handling` +- [x] - `p1` - `display_heading`: render a retrieval section's heading for human display, substituting a readable label for the synthetic preamble section's `None` heading instead of the literal string "None" - `inst-ui-display-heading` - [x] - `p1` - Create a temporary stderr-bound logger handler with plain-message formatting for UI diagnostics - `inst-ui-stderr-handler` - [x] - `p1` - Emit one plain-text stderr message through the dedicated helper, allowing a logger-backed implementation internally, then close the handler - `inst-ui-stderr-emit` - [x] - `p1` - `relpath`: convert absolute path to cwd-relative path with fallback - `inst-ui-relpath` @@ -718,7 +724,13 @@ Enables users to install Studio globally, initialize it in any project with sens 4. [x] - `p1` - Redact `$HOME` to `~` recursively so no username is recorded - `inst-log-redact` 5. [x] - `p1` - Append one schema-versioned event (run_id, decision_id, event, command, payload); never raise into the caller; rotate by size; show a one-time notice - `inst-log-record` 6. [x] - `p1` - Typed record helpers — routing, dispatch, validation, review, escalation, and command invocation (exit code, duration, arg-shape) — that call the writer - `inst-log-api` + - [x] - `p1` - `record_read`: log one read-and-answer event (method, target, lines, tokens, source) in the one shared schema every JIT-retrieval method's real cost is measured in - `inst-log-read-wrapper` 7. [x] - `p1` - Read events back oldest-first (skipping unparseable lines) and summarise counts by event and run - `inst-log-read` + - [x] - `p1` - `summarize_reads`: aggregate logged `"read"` events into a per-method token/line/count table - `inst-log-summarize-reads` + +**Supporting**: +- [x] - `p1` - `cfs usage-report` CLI wrapper: aggregate `summarize()` and `summarize_reads()` into one payload - `inst-usage-report-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs usage-report` output - `inst-usage-report-cmd-format` ## 4. States (CDSL) diff --git a/architecture/features/traceability-validation.md b/architecture/features/traceability-validation.md index 25afc650..dcb6a2e2 100644 --- a/architecture/features/traceability-validation.md +++ b/architecture/features/traceability-validation.md @@ -22,6 +22,12 @@ - [Validate TOC](#validate-toc) - [TOC Utilities](#toc-utilities) - [Document Index](#document-index) + - [TF-IDF Scoring](#tf-idf-scoring) + - [OKF Bundle](#okf-bundle) + - [Atomic File I/O](#atomic-file-io) + - [Heading-Nav Search](#heading-nav-search) + - [JIT-Retrieval Cascade](#jit-retrieval-cascade) + - [Read Gate](#read-gate) - [Markdown Parsing Utilities](#markdown-parsing-utilities) - [Fixing Prompt Enrichment](#fixing-prompt-enrichment) - [Headings Contract Validation](#headings-contract-validation) @@ -458,18 +464,22 @@ The cache-validity fingerprint is deliberately metadata-only (`mtime` + file size via `Path.stat()`), never a content hash — the point of the cache is to avoid reading the file at all on a hit, and a content hash would defeat that by requiring the read it's meant to save. A build reads the content and -takes that fingerprint bracketed by a stat snapshot on each side, so the -fingerprint saved is provably the one that matches what was actually parsed -even if a write lands in the narrow window during the read. +takes that fingerprint bracketed by a stat snapshot on each side: when the +two snapshots agree, the fingerprint saved is provably the one that +matches what was actually parsed even if a write lands in the narrow +window during the read; under sustained contention past a bounded retry +limit, it falls back to the last read paired with its own trailing, +unverified fingerprint -- safe because that content will simply be +detected as stale again on the very next check, never silently wrong. 1. [x] - `p1` - Build a fresh structural index: parse headings + line ranges from current content, compute the stat-based fingerprint, stamp the current schema version - `inst-doc-index-build` 2. [x] - `p1` - Load a cached index for a file, validated against current stat metadata (no content read on a hit) and against the required-field shape at the current schema version; returns `None` if missing, stale, corrupt, or an incomplete/outdated shape - `inst-doc-index-load` 3. [x] - `p1` - Persist an index to its cache location atomically (temp file + `os.replace`, so a concurrent reader never observes a torn write); no-ops silently outside a Studio-adapted project - `inst-doc-index-save` 4. [x] - `p1` - Return the cached index or build-and-cache a fresh one; reports cache hit/miss for benchmarking - `inst-doc-index-get-or-build` -5. [x] - `p1` - Attach a one-line, LLM-authored summary to a cached section by its `line_start`, for a future per-section-summary caller - `inst-doc-index-annotate` +5. [x] - `p1` - Attach a one-line, LLM-authored summary to a cached section by its `line_start`, only once the caller's `expected_hash` matches that section's current hash -- rejecting the write on mismatch instead of silently attaching the summary to whatever content now occupies that position - `inst-doc-index-annotate` 6. [x] - `p1` - Infer which heading level represents one retrievable section: the most-recurring level wins over a level that appears only once (however shallow), since PDF-conversion heading levels don't reliably encode true nesting depth — a fixed level assumption silently produces a degenerate mega-section on such documents - `inst-doc-index-infer-level` -7. [x] - `p1` - Group headings at exactly the inferred level into retrieval sections (off-level headings stay inside whichever section they fall under, never split one apart); hash each section's own text for section-granularity staleness detection - `inst-doc-index-retrieval-sections` -8. [x] - `p1` - Diff the current file against its last cached build at section granularity: which retrieval sections are unchanged vs. changed, or whether the section count itself changed (a structural change, matched by position not heading text, since duplicate titles are real) - `inst-doc-index-diff-stale` +7. [x] - `p1` - Group headings at exactly the inferred level into retrieval sections (off-level headings stay inside whichever section they fall under, never split one apart); hash each section's own text, trailing-whitespace-stripped per line, for section-granularity staleness detection; flag a section with nothing between it and the next same-level heading as `empty`; when real content (not just blank lines) precedes the first section-level heading, capture it as a leading synthetic entry (`heading=None`) instead of leaving it invisible to every entry here - `inst-doc-index-retrieval-sections` +8. [x] - `p1` - Diff the current file against its last cached build at section granularity: which retrieval sections are unchanged vs. changed, or whether the section count itself changed (a structural change). Matched primarily by content hash (a multiset match, so duplicate-content sections pair up correctly) rather than position, so a pure reorder with no text edits reports every section unchanged instead of misreporting the whole document as edited; a returned entry's `line_start` (not its hash) is what a caller uses to address "this specific section" afterwards, since duplicate heading titles are real - `inst-doc-index-diff-stale` **Supporting**: - [x] - `p1` - Stat-based cache-validity fingerprint (`mtime_ns` + size); resolved from the file's own path, never a content hash - `inst-doc-index-etag` @@ -478,6 +488,113 @@ even if a write lands in the narrow window during the read. - [x] - `p1` - Human-friendly formatter for `cfs doc-index` output - `inst-doc-index-cmd-format` - [x] - `p1` - Read a file's content bracketed by an etag snapshot on each side, retrying on mismatch: closes the window where a write between the read and the fingerprint could save stale headings under a fresh-looking etag - `inst-doc-index-stable-read` - [x] - `p1` - Re-parse a file's current content into retrieval sections for staleness comparison, and build the `(heading, line_start)` identity pair that disambiguates a duplicate heading title in a diff result - `inst-doc-index-diff-stale-helpers` +- [x] - `p1` - Slice a retrieval section's own raw text out of a file's lines by its `line_start`/`line_end` -- the one shared implementation every consumer needing a section's actual content (not just its boundaries) reuses instead of re-deriving the slice - `inst-doc-index-section-text` +- [x] - `p1` - `cfs doc-index` CLI wrapper: parse arguments, build the JSON output payload - `inst-doc-index-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs doc-index` output - `inst-doc-index-cmd-format` + +### TF-IDF Scoring + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-tfidf` + +**Input**: Markdown file path, query text + +**Output**: Retrieval sections ranked by TF-IDF score against the query, plus a confidence signal + +Purely mechanical, no LLM call: reuses the Document Index's `retrieval_sections` for section boundaries (read once per file, same as every other JIT-retrieval consumer), then scores each section as sum(term-frequency x inverse-document-frequency) over the query's own terms. A rare, distinctive term scores its one relevant section far above every other (verified against a real document: 0.0016 vs. 0.0000 everywhere else); a common term whose real answer lives in a longer, more thoroughly-covered section can still lose to a shorter section with a single coincidental mention, since term frequency is normalized by section length — a real, measured, and documented failure mode of this method on its own, not a defect in the implementation (see the "zero-shot" case in the source findings document: margin 1.06x, wrong section on top). This is exactly why a margin/unambiguous confidence signal is returned alongside the ranking rather than just the ranking alone — a routing layer built on top of this needs to know when the ranking itself isn't trustworthy, not just what it is. + +1. [x] - `p1` - Tokenize text: lowercase, alphanumeric-only, dropping tokens shorter than 3 characters - `inst-tfidf-tokenize` +2. [x] - `p1` - Score every retrieval section against a query: build the document's inverse-document-frequency table, rank sections by term-frequency x idf, and compute a margin/unambiguous confidence signal from the top two scores - `inst-tfidf-score` + +**Supporting**: +- [x] - `p1` - Inverse-document-frequency table builder, section ranker, and margin/unambiguous confidence calculator - `inst-tfidf-score-helpers` +- [x] - `p1` - `cfs tfidf-score` CLI wrapper: parse arguments, build the JSON output payload - `inst-tfidf-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs tfidf-score` output - `inst-tfidf-cmd-format` + +### OKF Bundle + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-okf` + +**Input**: Markdown file path; a written section's `line_start`, description, and body text (from an external caller) + +**Output**: A local, regenerable bundle of concept files + `index.md`, and a per-section missing/stale/current status report + +Deterministic infrastructure only, matching `doc_index.py`/`tfidf.py`: no LLM call happens in this module. Writing an actual section summary is an external caller's job (an agent, dispatched outside this codebase) -- this module tracks which concept files should exist relative to the document's *current* retrieval sections, detects when a written one is stale (its recorded `built_from_hash` no longer matches the section's current hash from the Document Index), and persists whatever the caller writes. The whole bundle is local-only and gitignored (`.cache/okf/` — see `.gitignore`): unlike the content of a summary, which is expensive to regenerate (real LLM tokens), the bundle not surviving a fresh clone just means it rebuilds from scratch the same way `doc_index.py`'s own cache does — nothing here assumes it survives across clones, only across calls on the same machine. + +1. [x] - `p1` - Resolve the local bundle directory for a source file within its Studio directory, resolved from the file's own path - `inst-okf-bundle-dir` +2. [x] - `p1` - Load/persist the bundle manifest (`manifest.json`) atomically; loading validates the decoded shape (a dict, an `entries` list, each entry carrying `line_start`/`concept_file`/`built_from_hash`), treating a malformed/pre-schema manifest as absent rather than returned broken - `inst-okf-manifest-io` +3. [x] - `p1` - Report the bundle's state against the document's *current* retrieval sections: missing (never summarized, or its concept file was deleted out from under it), stale (source changed since summary was written), or current. Matched primarily by content *hash*, not `line_start` -- inserting or reordering other sections shifts `line_start` without touching a section's own text, so a manifest entry is reconciled to whichever current section now carries its recorded hash (consumed one at a time per hash, preserving that entry's own `concept_file`) before falling back to `line_start` to distinguish a genuine edit (stale) from never-summarized (missing) - `inst-okf-status` +4. [x] - `p1` - Write (or overwrite) one section's concept file (YAML frontmatter values safely quoted against embedded colons/quotes/newlines) and its manifest entry under one exclusive lock spanning the whole read-modify-write-and-reindex cycle, then regenerate `index.md` from the bundle's real current status (not the raw manifest), so a deleted concept file drops out instead of becoming a dead link and a stale entry is visibly marked - `inst-okf-write-concept` + +**Supporting**: +- [x] - `p1` - YAML double-quoted-scalar escaper for frontmatter values, safe against caller-supplied content (an LLM's own summary text) containing colons, quotes, backslashes, or an embedded block-close sequence - `inst-okf-yaml-quote` +- [x] - `p1` - Concept-file frontmatter builder: title/description/resource/generated-by, every value safely quoted - `inst-okf-build-frontmatter` +- [x] - `p1` - Deterministic `index.md` template driven by real per-section status (missing/stale/current), not just the raw manifest, so it never disagrees with `cfs okf-status` - `inst-okf-render-index` +- [x] - `p1` - `cfs okf-status` CLI wrapper: parse arguments, build the JSON output payload - `inst-okf-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs okf-status` output, including the concept file path - `inst-okf-cmd-format` + +### Atomic File I/O + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-atomic-io` + +**Input**: A file path and content to write; a lock path and a read-modify-write callback + +**Output**: A file written without any observable torn/partial state; a callback run with cross-call exclusivity + +Shared by every local cache/bundle writer in this package (`doc_index.py`, `okf.py`) once a second consumer needed the exact same two behaviors a first implementation had already solved once -- extracted rather than reimplemented a second time. Mirrors the fallback shape `decision_log.py`'s own locking already established for this codebase (exclusive `fcntl` lock where available, unlocked elsewhere), kept separate since that module also bakes in log-rotation behavior these callers don't need. + +1. [x] - `p1` - Write text to a path atomically: temp file + `os.replace`, so a reader racing a concurrent writer sees either the old complete file or the new complete one, never a torn write - `inst-atomic-write` +2. [x] - `p1` - Run a read-modify-write callback under an exclusive lock on a sibling lock file, serializing concurrent callers so two overlapping cycles can't each read the same base state and have whichever writes last silently discard the other's update - `inst-atomic-lock` + +### Heading-Nav Search + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-heading-nav` + +**Input**: Markdown file path, query text + +**Output**: Every retrieval section containing the query literally, plus the first (document-order) hit + +Purely mechanical, no LLM call: a case-insensitive literal-substring search of the query against each retrieval section's own raw text, mirroring a real `grep -i ""` against the content -- deliberately not tokenized or word-split, and sharing the Document Index's `retrieval_sections` for boundaries so this reads no more of the file than every other JIT-retrieval consumer already does. Has no semantic fallback by design: a query phrased differently than the source's own vocabulary returns zero hits everywhere, even when a related concept exists under different wording (a real, documented failure mode of this method on its own, not a defect) -- that hard failure is itself the useful signal a caller needs to decide whether to escalate past this method. + +1. [x] - `p1` - Find every retrieval section containing a query's literal text (case-insensitive), in document order, plus the first match; excludes fenced code blocks from the match so an incidental code-sample hit can't count as a prose match - `inst-heading-nav-search` + +**Supporting**: +- [x] - `p1` - Blank out fenced-code-block lines before counting hits, reusing `toc.py`'s own fence-tracking - `inst-heading-nav-strip-fences` +- [x] - `p1` - `cfs heading-nav` CLI wrapper: parse arguments, build the JSON output payload - `inst-heading-nav-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs heading-nav` output - `inst-heading-nav-cmd-format` + +### JIT-Retrieval Cascade + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-cascade` + +**Input**: Markdown file path, query text; optional numeric margin threshold and expected future query volume + +**Output**: A routing decision -- resolved at Tier 1, resolved with multiple candidates, or escalated to a Tier 2 OKF-vs-baseline recommendation (plus a large-read gate check when that recommendation is baseline) + +Combines Heading-Nav Search and TF-IDF Scoring into the two-tier routing decision neither mechanical method answers on its own (see constructorfabric/studio#104): heading-nav's zero-hit case and TF-IDF's own agreement/margin against heading-nav's pick determine whether a query resolves for free at Tier 1, needs both methods' candidates read, or must escalate to a Tier 2 choice between the local OKF bundle and a full baseline read. Tier 1's large-margin resolution is deliberately restricted to TF-IDF's `unambiguous` signal rather than a numeric cutoff -- of the two real margins measured while designing this cascade, only an infinite (unambiguous) one was on a correct pick; every finite margin measured, however large, was on a documented wrong pick -- so a numeric `margin_threshold` exists as an explicit, off-by-default opt-in rather than a built-in assumption. Tier 2 never recommends an OKF concept file it knows is stale or missing for the section Tier 1 named as its escalation candidate: since nothing in this codebase can perform a background rebuild (no job runner, and by design no LLM call anywhere in this module or the ones it composes), falling back to baseline is the only choice that doesn't risk silently serving a known-wrong summary. + +1. [x] - `p1` - Apply the Tier 1 routing table: heading-nav zero hits escalates; agreement with TF-IDF's top pick resolves when unambiguous (or past an explicit margin threshold), else escalates as a diffuse margin; disagreement between the two resolves with both candidates - `inst-cascade-tier1` +2. [x] - `p1` - Choose OKF vs. baseline once Tier 1 escalates: an available bundle with no summarized sections yet counts as "no usable bundle"; otherwise every section that could actually be selected -- Tier 1's named candidate, or the whole bundle when there's no candidate to narrow to -- must be current, else recommend baseline with a rebuild flag - `inst-cascade-tier2` +3. [x] - `p1` - Route one query end to end: run Tier 1, and only when it escalates run Tier 2 and -- if Tier 2 recommends baseline -- the large-read confirmation gate against the document's real line count - `inst-cascade-route` + +**Supporting**: +- [x] - `p1` - `cfs retrieve` CLI wrapper: parse arguments, build the JSON output payload - `inst-cascade-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs retrieve` output - `inst-cascade-cmd-format` + +### Read Gate + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-read-gate` + +**Input**: A document's total line count; an optional line-count threshold + +**Output**: `{needs_confirmation, total_lines, threshold}` -- a structured verdict for an external caller to act on, not an interactive prompt + +Pure decision logic, no I/O: this is a deterministic CLI, not the caller that actually reads a file and answers a query, so it produces the structured flag that decision depends on rather than blocking on its own `input()` call. The default threshold (5,000 lines) is the real number measured during this design's own token-tracking prototype -- the only read among nine real candidate targets against a 166-page source document that crossed it was a whole-document baseline read. + +1. [x] - `p1` - Decide whether a read of a given line count should pause for confirmation against a threshold - `inst-read-gate-check` + +**Supporting**: +- [x] - `p1` - `cfs read-gate` CLI wrapper: build the document index, extract its total line count, apply the gate check - `inst-read-gate-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs read-gate` output - `inst-read-gate-cmd-format` ### Markdown Parsing Utilities @@ -745,6 +862,9 @@ The system **MUST** scan CDSL instruction markers (`inst-{slug}` suffixes in num | Fixing Utils | `skills/.../utils/fixing.py` | Fixing prompt generation for LLM agents | | Language Config | `skills/.../utils/language_config.py` | Language-specific file extensions and comment patterns | | Parsing Utils | `skills/.../utils/parsing.py` | Markdown structure parsing, section extraction | +| Heading-Nav Utils | `skills/.../utils/heading_nav.py` | Literal-substring section search for JIT retrieval | +| Cascade Utils | `skills/.../utils/cascade.py` | Two-tier JIT-retrieval routing (heading-nav + TF-IDF, OKF vs. baseline) | +| Read Gate Utils | `skills/.../utils/read_gate.py` | Large-read confirmation threshold check | ## 7. Acceptance Criteria diff --git a/skills/studio/scripts/studio/cli.py b/skills/studio/scripts/studio/cli.py index 633509c2..9febab0f 100644 --- a/skills/studio/scripts/studio/cli.py +++ b/skills/studio/scripts/studio/cli.py @@ -138,6 +138,30 @@ def _cmd_doc_index(argv: List[str]) -> int: from .commands.doc_index import cmd_doc_index return cmd_doc_index(argv) +def _cmd_tfidf_score(argv: List[str]) -> int: + from .commands.tfidf import cmd_tfidf_score + return cmd_tfidf_score(argv) + +def _cmd_okf_status(argv: List[str]) -> int: + from .commands.okf import cmd_okf_status + return cmd_okf_status(argv) + +def _cmd_heading_nav(argv: List[str]) -> int: + from .commands.heading_nav import cmd_heading_nav + return cmd_heading_nav(argv) + +def _cmd_retrieve(argv: List[str]) -> int: + from .commands.cascade import cmd_retrieve + return cmd_retrieve(argv) + +def _cmd_read_gate(argv: List[str]) -> int: + from .commands.read_gate import cmd_read_gate + return cmd_read_gate(argv) + +def _cmd_usage_report(argv: List[str]) -> int: + from .commands.usage_report import cmd_usage_report + return cmd_usage_report(argv) + # ============================================================================= # ADAPTER COMMAND # ============================================================================= @@ -221,6 +245,12 @@ def _cmd_map(argv: List[str]) -> int: "toc": "Generate/update Table of Contents", "chunk-input": "Chunk oversized workflow input into line-bounded Markdown files", "doc-index": "Build/reuse a cached heading index for a Markdown file (read once, not per query)", + "tfidf-score": "Rank a Markdown file's retrieval sections against a query via TF-IDF", + "okf-status": "Report an OKF bundle's state for a Markdown file (missing/stale/current per section)", + "heading-nav": "Find a Markdown file's retrieval sections containing a query's literal text", + "retrieve": "Route a query through the two-tier JIT-retrieval cascade (heading-nav + TF-IDF, OKF vs. baseline)", + "read-gate": "Check whether a Markdown file's line count crosses the large-read confirmation threshold", + "usage-report": "Aggregate the local decision log's read events into a per-method token table", "pdsl": "Validate PDSL prompt blocks", "workspace-init": "Initialize multi-repo workspace", "workspace-add": "Add a source to workspace config", @@ -237,7 +267,10 @@ def _cmd_map(argv: List[str]) -> int: ("Validation", ["validate", "validate-kits", "validate-toc", "spec-coverage", "check-language"]), ("Search & Navigation", ["list-ids", "list-id-kinds", "get-content", "where-defined", "where-used"]), ("Kit Management", ["kit"]), - ("Utility", ["toc", "chunk-input", "doc-index", "pdsl"]), + ("Utility", [ + "toc", "chunk-input", "doc-index", "tfidf-score", "okf-status", + "heading-nav", "retrieve", "read-gate", "usage-report", "pdsl", + ]), ("Workspace", ["workspace-init", "workspace-add", "workspace-info", "workspace-sync"]), ("Delegation", ["delegate"]), ("Diagnostics", ["doctor"]), @@ -269,6 +302,12 @@ def _cmd_map(argv: List[str]) -> int: "spec-coverage": "_cmd_spec_coverage", "chunk-input": "_cmd_chunk_input", "doc-index": "_cmd_doc_index", + "tfidf-score": "_cmd_tfidf_score", + "okf-status": "_cmd_okf_status", + "heading-nav": "_cmd_heading_nav", + "retrieve": "_cmd_retrieve", + "read-gate": "_cmd_read_gate", + "usage-report": "_cmd_usage_report", "workspace-init": "_cmd_workspace_init", "workspace-add": "_cmd_workspace_add", "workspace-info": "_cmd_workspace_info", @@ -303,6 +342,12 @@ def _cmd_map(argv: List[str]) -> int: _cmd_spec_coverage, _cmd_chunk_input, _cmd_doc_index, + _cmd_tfidf_score, + _cmd_okf_status, + _cmd_heading_nav, + _cmd_retrieve, + _cmd_read_gate, + _cmd_usage_report, _cmd_workspace_init, _cmd_workspace_add, _cmd_workspace_info, diff --git a/skills/studio/scripts/studio/commands/cascade.py b/skills/studio/scripts/studio/commands/cascade.py new file mode 100644 index 00000000..518997a3 --- /dev/null +++ b/skills/studio/scripts/studio/commands/cascade.py @@ -0,0 +1,96 @@ +"""Studio retrieve command — route a query against a Markdown file through +the two-tier JIT-retrieval cascade (heading-nav + TF-IDF, falling back to an +OKF-vs-baseline choice), and report the routing decision. + +Thin CLI wrapper around ``studio.utils.cascade``. + +@cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 +""" + +import argparse +import math +from typing import List + +from ..utils.cascade import route_query +from ..utils.ui import ui + + +def _margin_threshold_arg(value: str) -> float: + """argparse type for --margin-threshold: a finite number > 0. + + cascade.py's own module docstring states the design basis: the only + real evidence measured for this design is that an *infinite* margin is + safe, while finite margins of 1.06x-1.58x still occurred on two + independently wrong picks -- no finite value is yet proven safe. A + non-finite (nan/inf), negative, or zero threshold would make + ``tfidf_result["margin"] >= margin_threshold`` fire on virtually any + result, defeating that safety margin entirely; only a genuine positive + finite number is accepted, mirroring ``commands/eval.py``'s + ``_compliance_arg`` validator for the same class of hazard. + """ + try: + parsed = float(value) + except ValueError as exc: + raise argparse.ArgumentTypeError(f"invalid float value: {value!r}") from exc + if not math.isfinite(parsed) or parsed <= 0: + raise argparse.ArgumentTypeError( + f"--margin-threshold must be a finite number > 0, got {value!r}") + return parsed + + +# @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd +def cmd_retrieve(argv: List[str]) -> int: + """Route a query against a Markdown file through the JIT-retrieval cascade.""" + p = ui.JsonSafeArgumentParser( + prog="cfs retrieve", + description="Route a query through the two-tier JIT-retrieval cascade and report the decision.", + ) + p.add_argument("file", help="Markdown file path") + p.add_argument("query", help="Query text") + p.add_argument( + "--margin-threshold", type=_margin_threshold_arg, default=None, + help="Enable a numeric TF-IDF margin cutoff for a large-margin Tier 1 resolution " + "(default: disabled -- only an unambiguous score counts). Must be a finite number > 0.", + ) + p.add_argument( + "--expected-future-queries", type=int, default=None, + help="Expected future query volume against this document, for the OKF-vs-baseline break-even math", + ) + args, filepath = ui.parse_file_command(p, argv) + if filepath is None: + return 2 + + result, rc = ui.call_with_read_error_handling( + filepath, + lambda: route_query( + filepath, args.query, + margin_threshold=args.margin_threshold, + expected_future_queries=args.expected_future_queries, + ), + ) + if rc is not None: + return rc + + output = {"file": str(filepath), **result} + ui.result(output, human_fn=_human_retrieve) + return 0 +# @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd + + +# @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd-format +def _human_retrieve(data: dict) -> None: + ui.header("Retrieve") + ui.substep(f"query: {data['query']!r}") + ui.substep(f"tier: {data['tier']} ({data['reason']})") + for c in data["candidates"]: + ui.substep(f" [{c['line_start']}-{c['line_end']}] {c['heading']}") + if "tier2" in data: + tier2 = data["tier2"] + ui.substep(f"tier 2 recommendation: {tier2['recommendation']} ({tier2['reason']})") + if tier2.get("okf_needs_rebuild"): + ui.substep(" OKF bundle exists but is stale/missing for this candidate -- needs a rebuild") + if "read_gate" in data and data["read_gate"]["needs_confirmation"]: + gate = data["read_gate"] + ui.substep(f"read gate: needs confirmation ({gate['total_lines']} lines > {gate['threshold']})") + ui.blank() +# @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-cmd-format diff --git a/skills/studio/scripts/studio/commands/doc_index.py b/skills/studio/scripts/studio/commands/doc_index.py index ba512b50..0987d739 100644 --- a/skills/studio/scripts/studio/commands/doc_index.py +++ b/skills/studio/scripts/studio/commands/doc_index.py @@ -5,9 +5,7 @@ Thin CLI wrapper around ``studio.utils.doc_index``. """ -import argparse import logging -from pathlib import Path from typing import List from ..utils.doc_index import get_or_build_doc_index @@ -19,7 +17,7 @@ # @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd def cmd_doc_index(argv: List[str]) -> int: """Build (or reuse the cached) structural index for a Markdown file.""" - p = argparse.ArgumentParser( + p = ui.JsonSafeArgumentParser( prog="cfs doc-index", description=( "Build or reuse a cached heading/section index for a Markdown file, " @@ -35,24 +33,15 @@ def cmd_doc_index(argv: List[str]) -> int: action="store_true", help="Force a fresh build even if a valid cached index exists", ) - args = p.parse_args(argv) - - filepath = Path(args.file).resolve() - if not filepath.is_file(): - ui.result( - {"file": str(filepath), "status": "ERROR", "message": "File not found"}, - human_fn=lambda d: ui.error(f"{d['file']}: {d['message']}"), - ) + args, filepath = ui.parse_file_command(p, argv) + if filepath is None: return 2 try: index = get_or_build_doc_index(filepath, force_rebuild=args.rebuild) except (OSError, UnicodeDecodeError) as exc: logger.warning("doc-index: cannot read %s: %s", filepath, exc) - ui.result( - {"file": str(filepath), "status": "ERROR", "message": f"Cannot read file: {exc}"}, - human_fn=lambda d: ui.error(f"{d['file']}: {d['message']}"), - ) + ui.report_read_error(filepath, exc) return 2 output = { @@ -87,6 +76,7 @@ def _human_doc_index(data: dict) -> None: else "Retrieval sections (no headings — none inferred)") for s in data["retrieval_sections"]: summary = f" — {s['summary']}" if s.get("summary") else "" - ui.substep(f" [{s['line_start']}-{s['line_end']}] {s['heading']} ({s['hash'][:12]}){summary}") + heading = ui.display_heading(s["heading"]) + ui.substep(f" [{s['line_start']}-{s['line_end']}] {heading} ({s['hash'][:12]}){summary}") ui.blank() # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd-format diff --git a/skills/studio/scripts/studio/commands/heading_nav.py b/skills/studio/scripts/studio/commands/heading_nav.py new file mode 100644 index 00000000..587d5009 --- /dev/null +++ b/skills/studio/scripts/studio/commands/heading_nav.py @@ -0,0 +1,55 @@ +"""Studio heading-nav command — grep a Markdown file's retrieval sections +for a query's literal text, for inspecting/benchmarking the JIT-retrieval +mechanical gate independent of any cascade routing logic built on top of it. + +Thin CLI wrapper around ``studio.utils.heading_nav``. + +@cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 +""" + +from typing import List + +from ..utils.heading_nav import find_sections +from ..utils.ui import ui + + +# @cpt-begin:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd +def cmd_heading_nav(argv: List[str]) -> int: + """Find a Markdown file's retrieval sections containing a query literally.""" + p = ui.JsonSafeArgumentParser( + prog="cfs heading-nav", + description="Find a Markdown file's retrieval sections containing a query's literal text.", + ) + p.add_argument("file", help="Markdown file path") + p.add_argument("query", help="Query text to search for, literally") + args, filepath = ui.parse_file_command(p, argv) + if filepath is None: + return 2 + + result, rc = ui.call_with_read_error_handling(filepath, lambda: find_sections(filepath, args.query)) + if rc is not None: + return rc + + output = { + "file": str(filepath), + "query": args.query, + "matches": result["matches"], + "first_match": result["first_match"], + } + ui.result(output, human_fn=_human_heading_nav) + return 0 +# @cpt-end:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd + + +# @cpt-begin:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd-format +def _human_heading_nav(data: dict) -> None: + ui.header("Heading-Nav Search") + ui.substep(f"query: {data['query']!r}") + if not data["matches"]: + ui.substep("(no matches -- this method has no semantic fallback)") + ui.blank() + return + for entry in data["matches"]: + ui.substep(f" {entry['hit_count']}x [{entry['line_start']}-{entry['line_end']}] {entry['heading']}") + ui.blank() +# @cpt-end:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-cmd-format diff --git a/skills/studio/scripts/studio/commands/okf.py b/skills/studio/scripts/studio/commands/okf.py new file mode 100644 index 00000000..74d59309 --- /dev/null +++ b/skills/studio/scripts/studio/commands/okf.py @@ -0,0 +1,60 @@ +"""Studio okf-status command — report an OKF bundle's state for a Markdown +file: which concept files exist, are stale, or are missing entirely, +relative to the document's current retrieval sections. + +Read-only. Writing a concept file is an external caller's job (an agent +that has actually produced a summary) via +``studio.utils.okf.write_concept_file`` -- this command never invokes an +LLM itself. + +Thin CLI wrapper around ``studio.utils.okf``. +""" + +from typing import List + +from ..utils.okf import get_okf_status +from ..utils.ui import ui + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd +def cmd_okf_status(argv: List[str]) -> int: + """Report an OKF bundle's state for a Markdown file.""" + p = ui.JsonSafeArgumentParser( + prog="cfs okf-status", + description="Report which OKF concept files exist, are stale, or are missing for a Markdown file.", + ) + p.add_argument("file", help="Markdown file path") + _args, filepath = ui.parse_file_command(p, argv) + if filepath is None: + return 2 + + status, rc = ui.call_with_read_error_handling(filepath, lambda: get_okf_status(filepath)) + if rc is not None: + return rc + output = {"file": str(filepath), **status} + ui.result(output, human_fn=_human_okf_status) + return 0 +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd-format +def _human_okf_status(data: dict) -> None: + ui.header("OKF Status") + if not data["available"]: + ui.substep("no Studio directory found -- OKF is unavailable for this file") + ui.blank() + return + ui.substep(f"bundle: {data['bundle_dir']}") + counts: dict = {} + for entry in data["entries"]: + counts[entry["status"]] = counts.get(entry["status"], 0) + 1 + summary = ", ".join(f"{count} {status}" for status, count in sorted(counts.items())) or "no sections" + ui.substep(summary) + for entry in data["entries"]: + heading = ui.display_heading(entry["heading"]) + ui.substep( + f" [{entry['status']:>7}] [{entry['line_start']}-{entry['line_end']}] " + f"{heading} -> {entry['concept_file']}" + ) + ui.blank() +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-cmd-format diff --git a/skills/studio/scripts/studio/commands/read_gate.py b/skills/studio/scripts/studio/commands/read_gate.py new file mode 100644 index 00000000..73dcdfef --- /dev/null +++ b/skills/studio/scripts/studio/commands/read_gate.py @@ -0,0 +1,53 @@ +"""Studio read-gate command — check whether a Markdown file's line count +crosses the large-read confirmation threshold, for a caller deciding +whether to pause before reading it in full. + +Thin CLI wrapper around ``studio.utils.read_gate``. + +@cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 +""" + +from typing import List + +from ..utils.doc_index import get_or_build_doc_index +from ..utils.read_gate import DEFAULT_GATE_THRESHOLD_LINES, check_gate +from ..utils.ui import ui + + +# @cpt-begin:cpt-studio-algo-traceability-validation-read-gate:p1:inst-read-gate-cmd +def cmd_read_gate(argv: List[str]) -> int: + """Check whether a Markdown file's line count needs read confirmation.""" + p = ui.JsonSafeArgumentParser( + prog="cfs read-gate", + description="Check whether a Markdown file's line count crosses the large-read confirmation threshold.", + ) + p.add_argument("file", help="Markdown file path") + p.add_argument( + "--threshold", type=int, default=DEFAULT_GATE_THRESHOLD_LINES, + help=f"Line-count threshold (default: {DEFAULT_GATE_THRESHOLD_LINES})", + ) + args, filepath = ui.parse_file_command(p, argv) + if filepath is None: + return 2 + + index, rc = ui.call_with_read_error_handling(filepath, lambda: get_or_build_doc_index(filepath)) + if rc is not None: + return rc + gate = check_gate(index["total_lines"], args.threshold) + + output = {"file": str(filepath), **gate} + ui.result(output, human_fn=_human_read_gate) + return 0 +# @cpt-end:cpt-studio-algo-traceability-validation-read-gate:p1:inst-read-gate-cmd + + +# @cpt-begin:cpt-studio-algo-traceability-validation-read-gate:p1:inst-read-gate-cmd-format +def _human_read_gate(data: dict) -> None: + ui.header("Read Gate") + ui.substep(f"{data['total_lines']} lines (threshold: {data['threshold']})") + if data["needs_confirmation"]: + ui.substep("needs confirmation -- this read crosses the threshold") + else: + ui.substep("no confirmation needed") + ui.blank() +# @cpt-end:cpt-studio-algo-traceability-validation-read-gate:p1:inst-read-gate-cmd-format diff --git a/skills/studio/scripts/studio/commands/tfidf.py b/skills/studio/scripts/studio/commands/tfidf.py new file mode 100644 index 00000000..e2431860 --- /dev/null +++ b/skills/studio/scripts/studio/commands/tfidf.py @@ -0,0 +1,72 @@ +"""Studio tfidf-score command — score a Markdown file's retrieval sections +against a query via TF-IDF, for inspecting/benchmarking the JIT-retrieval +mechanical gate independent of any cascade routing logic built on top of it. + +Thin CLI wrapper around ``studio.utils.tfidf``. +""" + +import logging +from typing import List + +from ..utils.tfidf import score_sections +from ..utils.ui import ui + +logger = logging.getLogger(__name__) + + +# @cpt-begin:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-cmd +def cmd_tfidf_score(argv: List[str]) -> int: + """Score a Markdown file's retrieval sections against a query via TF-IDF.""" + p = ui.JsonSafeArgumentParser( + prog="cfs tfidf-score", + description=( + "Rank a Markdown file's retrieval sections against a query via TF-IDF. " + "The result includes a margin/unambiguous confidence signal -- check it " + "before trusting the top-ranked section, since a nonzero score is never " + "a correctness guarantee on its own." + ), + ) + p.add_argument("file", help="Markdown file path") + p.add_argument("query", help="Query text to score sections against") + args, filepath = ui.parse_file_command(p, argv) + if filepath is None: + return 2 + + try: + result = score_sections(filepath, args.query) + except (OSError, UnicodeDecodeError) as exc: + logger.warning("tfidf-score: cannot read %s: %s", filepath, exc) + ui.report_read_error(filepath, exc) + return 2 + + output = { + "file": str(filepath), + "query": args.query, + "margin": result["margin"], + "unambiguous": result["unambiguous"], + "ranked": result["ranked"], + } + ui.result(output, human_fn=_human_tfidf_score) + return 0 +# @cpt-end:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-cmd + + +# @cpt-begin:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-cmd-format +def _human_tfidf_score(data: dict) -> None: + ui.header("TF-IDF Score") + ui.substep(f"query: {data['query']!r}") + if not data["ranked"]: + ui.substep("(no retrieval sections in this document)") + ui.blank() + return + if data["unambiguous"]: + ui.substep("confidence: unambiguous (top score positive, every other section scores 0)") + elif data["margin"] is not None: + ui.substep(f"confidence: margin {data['margin']:.2f}x over the runner-up") + else: + ui.substep("confidence: none (top score is 0 -- no query term matched anywhere)") + for entry in data["ranked"]: + heading = ui.display_heading(entry["heading"]) + ui.substep(f" {entry['score']:.6f} [{entry['line_start']}-{entry['line_end']}] {heading}") + ui.blank() +# @cpt-end:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-cmd-format diff --git a/skills/studio/scripts/studio/commands/usage_report.py b/skills/studio/scripts/studio/commands/usage_report.py new file mode 100644 index 00000000..75bc3e5c --- /dev/null +++ b/skills/studio/scripts/studio/commands/usage_report.py @@ -0,0 +1,63 @@ +"""Studio usage-report command — aggregate the local decision log's logged +read events into a per-method token table, and a summary of everything else +logged. + +Thin CLI wrapper around ``studio.utils.decision_log``. + +@cpt-flow:cpt-studio-flow-traceability-validation-validate:p1 +""" + +from typing import List + +from ..utils import decision_log +from ..utils.ui import ui + + +# @cpt-begin:cpt-studio-algo-core-infra-decision-log:p1:inst-usage-report-cmd +def cmd_usage_report(argv: List[str]) -> int: + """Aggregate the local decision log into a per-method usage report.""" + p = ui.JsonSafeArgumentParser( + prog="cfs usage-report", + description="Aggregate the local decision log's read events into a per-method token table.", + ) + if ui.parse_args_or_json_error(p, argv) is None: + return 2 + + output = { + "summary": decision_log.summarize(), + "reads": decision_log.summarize_reads(), + } + ui.result(output, human_fn=_human_usage_report) + return 0 +# @cpt-end:cpt-studio-algo-core-infra-decision-log:p1:inst-usage-report-cmd + + +# @cpt-begin:cpt-studio-algo-core-infra-decision-log:p1:inst-usage-report-cmd-format +def _human_usage_report(data: dict) -> None: + ui.header("Usage Report") + summary = data["summary"] + if not summary["exists"]: + ui.substep("no decision log found yet") + ui.blank() + return + ui.substep(f"log: {summary['path']}") + ui.substep(f"{summary['total_events']} event(s) across {summary['runs']} run(s)") + if summary["first_ts"] or summary["last_ts"]: + ui.substep(f"time range: {summary['first_ts']} .. {summary['last_ts']}") + if summary["event_counts"]: + counts = ", ".join(f"{name}: {count}" for name, count in sorted(summary["event_counts"].items())) + ui.substep(f"by event type: {counts}") + + reads = data["reads"] + if not reads["methods"]: + ui.substep("no read events logged yet") + ui.blank() + return + ui.step(f"By method ({reads['total_tokens']} tokens total)") + for method, stats in sorted(reads["methods"].items()): + ui.substep( + f" {method}: {stats['count']} read(s), " + f"{stats['total_tokens']} tokens, {stats['total_lines']} lines" + ) + ui.blank() +# @cpt-end:cpt-studio-algo-core-infra-decision-log:p1:inst-usage-report-cmd-format diff --git a/skills/studio/scripts/studio/commands/validate_toc.py b/skills/studio/scripts/studio/commands/validate_toc.py index d1e49ebd..12f98cb2 100644 --- a/skills/studio/scripts/studio/commands/validate_toc.py +++ b/skills/studio/scripts/studio/commands/validate_toc.py @@ -151,6 +151,8 @@ def _human_validate_toc(data: dict) -> None: ui.warn(f"{path}: {warns} warning(s)") for w in r.get("warnings", []): ui.substep(f" ⚠ {w}") + elif status == "ERROR": + ui.error(f"{path}: {r.get('message', 'unknown error')}") else: ui.substep(f"{path}: {status}") overall = data.get("status", "") diff --git a/skills/studio/scripts/studio/utils/atomic_io.py b/skills/studio/scripts/studio/utils/atomic_io.py new file mode 100644 index 00000000..90b4fbc9 --- /dev/null +++ b/skills/studio/scripts/studio/utils/atomic_io.py @@ -0,0 +1,71 @@ +"""Filesystem primitives shared by every local cache/bundle writer in this +package: atomic replace-on-write, and cross-process exclusive locking +around a read-modify-write cycle. + +Extracted once a second consumer (``okf.py``, alongside ``doc_index.py``) +needed the exact same two behaviors, rather than reimplementing them a +second time. Mirrors the fallback shape ``decision_log.py``'s own +``_append_locked`` already established for this codebase (exclusive +``fcntl`` lock where available, unlocked elsewhere) -- kept separate from +that module since it also bakes in log-rotation behavior these two callers +don't need. + +@cpt-algo:cpt-studio-algo-traceability-validation-atomic-io:p1 +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path +from typing import Callable, TypeVar + +T = TypeVar("T") + + +# @cpt-begin:cpt-studio-algo-traceability-validation-atomic-io:p1:inst-atomic-write +def atomic_write_text(path: Path, content: str, *, encoding: str = "utf-8") -> None: + """Write ``content`` to ``path`` atomically: temp file + ``os.replace``, + so a reader racing a concurrent writer sees either the old complete + file or the new complete one, never a torn/partial write. + + The temp file gets a unique name per call (``tempfile.mkstemp``), not + just per-process (a PID-based name): two threads in the same process + writing the same target would otherwise share one temp path and race + each other's write/replace/cleanup. + """ + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp_name = tempfile.mkstemp(dir=path.parent, prefix=f".{path.name}.", suffix=".tmp") + tmp_path = Path(tmp_name) + try: + with os.fdopen(fd, "w", encoding=encoding) as tmp_fh: + tmp_fh.write(content) + os.replace(tmp_path, path) + except Exception: + tmp_path.unlink(missing_ok=True) + raise +# @cpt-end:cpt-studio-algo-traceability-validation-atomic-io:p1:inst-atomic-write + + +# @cpt-begin:cpt-studio-algo-traceability-validation-atomic-io:p1:inst-atomic-lock +def with_file_lock(lock_path: Path, fn: Callable[[], T]) -> T: + """Run ``fn()`` -- a read-modify-write cycle -- under an exclusive lock + on ``lock_path``, serializing concurrent callers so two overlapping + cycles against the same underlying resource can't each read the same + base state, mutate their own part, and have whichever writes last + silently discard the other's update. + + An exclusive ``fcntl`` lock where available (POSIX), otherwise runs + ``fn()`` unlocked (e.g. Windows) -- the atomicity of any individual + write is :func:`atomic_write_text`'s separate guarantee; only the + cross-call serialization is best-effort here. + """ + try: + import fcntl # pylint: disable=import-outside-toplevel + except ImportError: + return fn() + lock_path.parent.mkdir(parents=True, exist_ok=True) + with open(lock_path, "a", encoding="utf-8") as lock_fh: + fcntl.flock(lock_fh.fileno(), fcntl.LOCK_EX) + return fn() +# @cpt-end:cpt-studio-algo-traceability-validation-atomic-io:p1:inst-atomic-lock diff --git a/skills/studio/scripts/studio/utils/cascade.py b/skills/studio/scripts/studio/utils/cascade.py new file mode 100644 index 00000000..19344ee8 --- /dev/null +++ b/skills/studio/scripts/studio/utils/cascade.py @@ -0,0 +1,231 @@ +"""Two-tier JIT-retrieval cascade: combine heading-nav and TF-IDF into a +routing decision (Tier 1), falling back to an OKF-vs-baseline choice (Tier +2) only when Tier 1 can't resolve confidently on its own. + +Pure decision logic: never reads more of the document than heading-nav/ +TF-IDF/OKF status already needed, and never calls an LLM -- the actual +answering step (reading the picked section(s), or the whole document) stays +an external caller's job, same as every other module in this package. + +Tier 1 routing table (real evidence, see the design session's findings): + +| # | Pattern | Resolution | +|---|------------------------------------------------------|--------------------------| +| 1 | heading-nav: 0 hits | escalate | +| 2 | heading-nav>0, TF-IDF agrees, unambiguous | resolved (Tier 1) | +| 3 | heading-nav>0, TF-IDF disagrees | resolved_multi (Tier 1) | +| 4 | heading-nav>0, TF-IDF agrees, margin not unambiguous | escalate | + +Row 2's "large margin" is deliberately restricted to ``unambiguous=True`` +rather than a numeric margin cutoff: the only two real data points measured +for this design (an infinite margin on a correct pick, and 1.06x-1.58x +margins on two independently wrong picks) support "unambiguous is safe, +anything finite isn't yet proven safe" -- not a specific numeric threshold. +``margin_threshold`` exists so a numeric cutoff can be enabled later, once +there's real evidence for one, without an API change. + +Tier 2 never recommends an OKF concept file known to be stale/missing for +the candidate section Tier 1 identified (see :func:`route_tier2`) -- this is +this cascade's answer to the "does staleness block or serve-stale" design +question: since nothing in this codebase can perform a background rebuild +(there is no job runner, and by design no module here ever calls an LLM), +serving a known-stale OKF pointer would be a silent wrong answer with no +mechanism to ever correct itself. Falling back to baseline is the only +option that fits what this codebase can actually guarantee. + +@cpt-algo:cpt-studio-algo-traceability-validation-cascade:p1 +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any, Dict, Optional, cast + +from .doc_index import get_or_build_doc_index +from .heading_nav import find_sections +from .okf import get_okf_status +from .read_gate import check_gate +from .tfidf import score_sections + +#: Real, measured per-query token rates (see the design session's findings). +_OKF_BUILD_COST_TOKENS = 301_187 +_OKF_PER_QUERY_TOKENS = 45_735 +_BASELINE_PER_QUERY_TOKENS = 333_573 + + +def _as_candidate(section: Dict[str, Any]) -> Dict[str, Any]: + return { + "heading": section["heading"], + "line_start": section["line_start"], + "line_end": section["line_end"], + } + + +# @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-tier1 +def route_tier1(path: Path, query: str, *, margin_threshold: Optional[float] = None) -> Dict[str, Any]: + """Apply the Tier 1 routing table to a query against ``path``. + + Returns ``{"tier": "resolved" | "resolved_multi" | "escalate", "reason": + str, "candidates": [...]}``. ``candidates`` is the section(s) a caller + should actually read: one for rows 2/4 (row 4 despite escalating, since + it's still the best Tier-1 guess to hand Tier 2), two for row 3, none + for row 1 (heading-nav found nothing to anchor a guess to at all). + """ + nav_first_match = find_sections(path, query)["first_match"] + if nav_first_match is None: + return {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + # pylint's astroid inference traces find_sections()'s "matches[0] if matches + # else None" ternary and keeps treating this as Optional even after the + # None-check above narrows it -- a known astroid limitation across module + # boundaries (github.com/pylint-dev/pylint/issues/3162), not a real risk here. + nav_pick = cast(Dict[str, Any], nav_first_match) + + tfidf_result = score_sections(path, query) + # find_sections and score_sections both read retrieval_sections from the + # same get_or_build_doc_index(path) call: a heading-nav match guarantees + # at least one section exists, so TF-IDF always has one to rank too. + tfidf_pick = tfidf_result["ranked"][0] + + if tfidf_pick["line_start"] != nav_pick["line_start"]: # pylint: disable=unsubscriptable-object + return { + "tier": "resolved_multi", + "reason": "heading_nav_tfidf_disagree", + "candidates": [_as_candidate(nav_pick), _as_candidate(tfidf_pick)], + } + + agree_large_margin = tfidf_result["unambiguous"] or ( + margin_threshold is not None + and tfidf_result["margin"] is not None + and tfidf_result["margin"] >= margin_threshold + ) + if agree_large_margin: + return { + "tier": "resolved", + "reason": "heading_nav_tfidf_agree_large_margin", + "candidates": [_as_candidate(nav_pick)], + } + + return { + "tier": "escalate", + "reason": "diffuse_margin", + "candidates": [_as_candidate(nav_pick)], + } +# @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-tier1 + + +# @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-tier2 +def _baseline_recommendation(expected_future_queries: Optional[int]) -> Dict[str, Any]: + rec: Dict[str, Any] = {"recommendation": "baseline", "reason": "no_current_okf_bundle"} + if expected_future_queries is not None and expected_future_queries > 0: + okf_total = _OKF_BUILD_COST_TOKENS + _OKF_PER_QUERY_TOKENS * expected_future_queries + baseline_total = _BASELINE_PER_QUERY_TOKENS * expected_future_queries + rec["build_okf_break_even"] = { + "okf_total_tokens": okf_total, + "baseline_total_tokens": baseline_total, + "building_okf_would_pay_off": okf_total < baseline_total, + } + return rec + + +def route_tier2( + path: Path, + tier1_result: Dict[str, Any], + *, + expected_future_queries: Optional[int] = None, +) -> Dict[str, Any]: + """Choose OKF vs. baseline once Tier 1 has escalated. + + Only called for rows 1/4 (see :func:`route_tier1`). When Tier 1 named a + candidate section (row 4), only that section's concept file must be + current. Row 1 has no candidate section to narrow to -- heading-nav + found nothing, so the query could need any section -- and OKF's own + (external, LLM-driven) file-selection step picks among *whatever this + function hands it*; recommending OKF there while some other section is + stale or missing would let that external step land on exactly the + untrustworthy one. Either way, a stale or missing concept file in the + checked set downgrades the recommendation to baseline with + ``okf_needs_rebuild: True`` rather than risking a known-wrong summary -- + see this module's docstring for why that's the only coherent choice + here. + """ + status = get_okf_status(path) + # get_okf_status() returns one entry per retrieval section regardless of + # whether anything was ever summarized -- an "available" bundle_dir with + # every entry "missing" means no concept file has actually been written + # yet, which is "no bundle" for this decision, not "bundle exists." + bundle_exists = status["available"] and any(entry["status"] != "missing" for entry in status["entries"]) + if not bundle_exists: + return _baseline_recommendation(expected_future_queries) + + candidates = tier1_result.get("candidates", []) + if candidates: + by_line_start = {entry["line_start"]: entry for entry in status["entries"]} + relevant = [by_line_start.get(c["line_start"]) for c in candidates] + # A candidate that no longer maps to a current section (the + # document changed structurally between Tier 1 picking it and this + # re-derived status) can't be verified at all -- silently dropping + # it would let an all-unresolved candidate list pass the "any + # stale/missing" check below vacuously (empty list, no False + # values), recommending OKF on a candidate that was never actually + # checked. Treat "can't verify" the same as "not current". + if any(entry is None for entry in relevant): + rec = _baseline_recommendation(expected_future_queries) + rec["okf_needs_rebuild"] = True + return rec + else: + # No named candidate (row 1): the external selector could land on + # any section in the bundle, so every section must be trustworthy, + # not just some of them. + relevant = status["entries"] + + if any(entry["status"] != "current" for entry in relevant): + rec = _baseline_recommendation(expected_future_queries) + rec["okf_needs_rebuild"] = True + return rec + + return {"recommendation": "okf", "reason": "okf_bundle_current", "bundle_dir": status["bundle_dir"]} +# @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-tier2 + + +# @cpt-begin:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-route +def route_query( + path: Path, + query: str, + *, + margin_threshold: Optional[float] = None, + expected_future_queries: Optional[int] = None, +) -> Dict[str, Any]: + """Route one query end to end: Tier 1, then Tier 2 only if it escalates. + + When Tier 2 recommends baseline (a full-document read), also runs the + large-read confirmation gate against the document's real line count -- + the integration point this cascade exists to close, so a baseline + fallback never happens without the caller seeing whether it crosses the + confirmation threshold. + + Returned shape is a stable contract, not incidental: top-level ``query``, + ``tier``, ``reason``, ``candidates`` (:func:`route_tier1`'s own return, + merged in) are always present; ``tier2`` (:func:`route_tier2`'s return, + with ``recommendation``/``reason``/optional ``okf_needs_rebuild``) is + added only when Tier 1 escalated; ``read_gate`` + (:func:`studio.utils.read_gate.check_gate`'s return, with + ``needs_confirmation``/``total_lines``/``threshold``) is added only when + Tier 2 recommends ``"baseline"``. ``commands/cascade.py``'s + ``_human_retrieve`` and ``tests/test_cascade.py`` both key into these + fields by name -- changing a key here is a breaking change for both and + should be treated as one (versioned or coordinated), not a routine edit. + """ + tier1 = route_tier1(path, query, margin_threshold=margin_threshold) + result: Dict[str, Any] = {"query": query, **tier1} + if tier1["tier"] != "escalate": + return result + + tier2 = route_tier2(path, tier1, expected_future_queries=expected_future_queries) + result["tier2"] = tier2 + + if tier2["recommendation"] == "baseline": + index = get_or_build_doc_index(path) + result["read_gate"] = check_gate(index["total_lines"]) + + return result +# @cpt-end:cpt-studio-algo-traceability-validation-cascade:p1:inst-cascade-route diff --git a/skills/studio/scripts/studio/utils/decision_log.py b/skills/studio/scripts/studio/utils/decision_log.py index afc046b2..40302695 100644 --- a/skills/studio/scripts/studio/utils/decision_log.py +++ b/skills/studio/scripts/studio/utils/decision_log.py @@ -52,7 +52,7 @@ SCHEMA_VERSION = 1 #: Event names this module writes. Readers must tolerate others. -EVENTS = ("routing", "dispatch", "validation", "review", "escalation", "invocation") +EVENTS = ("routing", "dispatch", "validation", "review", "escalation", "invocation", "read") #: Environment overrides. _ENV_PATH = "CFS_DECISION_LOG" # explicit path, or an off-value to disable @@ -359,6 +359,20 @@ def record_invocation(command: str, exit_code: int = 0, duration_ms: int = 0, "exit_code": exit_code, "duration_ms": duration_ms, "args": dict(args_shape or {}), }, command=command, decision_id=decision_id, path=path) + + +# @cpt-begin:cpt-studio-algo-core-infra-decision-log:p1:inst-log-read-wrapper +def record_read(method: str, target: str, lines: int, tokens: int, source: str = "", *, + command: str = "", decision_id: str = "", + path: Optional[Path] = None) -> bool: + """Log one read-and-answer event: which retrieval method fired, and its + real cost, in the one shared schema every method's cost is measured in. + """ + return record("read", { + "method": method, "target": _redact(target), + "lines": lines, "tokens": tokens, "source": source, + }, command=command, decision_id=decision_id, path=path) +# @cpt-end:cpt-studio-algo-core-infra-decision-log:p1:inst-log-read-wrapper # @cpt-end:cpt-studio-algo-core-infra-decision-log:p1:inst-log-api @@ -440,4 +454,41 @@ def summarize(path: Optional[Path] = None) -> Dict[str, Any]: "first_ts": first_ts, "last_ts": last_ts, } + + +# @cpt-begin:cpt-studio-algo-core-infra-decision-log:p1:inst-log-summarize-reads +def summarize_reads(path: Optional[Path] = None) -> Dict[str, Any]: + """Aggregate logged ``"read"`` events into a per-method token table. + + Returns ``{"methods": {method: {"count", "total_tokens", "total_lines"}}, + "total_tokens": int}`` -- the per-method cost comparison a caller needs + to see which retrieval method is actually earning its keep on a real + document, not just how many events were logged. + + A record whose ``payload`` isn't a dict, or whose ``tokens``/``lines`` + values aren't numeric, is skipped rather than raising -- the same + "a partially corrupt log is still evidence" tolerance + :func:`read_events` already applies to unparseable lines, extended to a + parseable line with a malformed payload shape. + """ + methods: Dict[str, Dict[str, int]] = {} + total_tokens = 0 + for obj in read_events(path, event="read"): + payload = obj.get("payload") + if not isinstance(payload, dict): + continue + try: + tokens = int(payload.get("tokens", 0) or 0) + lines = int(payload.get("lines", 0) or 0) + except (TypeError, ValueError) as exc: + logger.debug("decision log: skipping read event with non-numeric tokens/lines: %s", exc) + continue + method = str(payload.get("method", "?")) + entry = methods.setdefault(method, {"count": 0, "total_tokens": 0, "total_lines": 0}) + entry["count"] += 1 + entry["total_tokens"] += tokens + entry["total_lines"] += lines + total_tokens += tokens + return {"methods": methods, "total_tokens": total_tokens} +# @cpt-end:cpt-studio-algo-core-infra-decision-log:p1:inst-log-summarize-reads # @cpt-end:cpt-studio-algo-core-infra-decision-log:p1:inst-log-read diff --git a/skills/studio/scripts/studio/utils/doc_index.py b/skills/studio/scripts/studio/utils/doc_index.py index f239b260..fb2d0b9e 100644 --- a/skills/studio/scripts/studio/utils/doc_index.py +++ b/skills/studio/scripts/studio/utils/doc_index.py @@ -34,12 +34,12 @@ import hashlib import json import logging -import os import time from collections import Counter from pathlib import Path from typing import Any, Dict, List, Optional, Tuple +from .atomic_io import atomic_write_text, with_file_lock from .toc import parse_headings_with_lines logger = logging.getLogger(__name__) @@ -173,26 +173,66 @@ def _build_retrieval_sections( :func:`diff_stale_sections` needs to tell "this one section changed" from "the whole file changed", which a whole-file fingerprint structurally cannot do. + + Content before the first ``section_level`` heading (a document title, + an intro paragraph) is otherwise invisible to every entry here, since + each entry starts at a heading line -- a real gap, since that region is + exactly where a title or one-line summary usually lives. When such + content exists and isn't just blank lines, it's captured as a leading + synthetic entry with ``heading=None`` (never a real heading's value, + so a caller can tell it apart from actual sections) spanning lines 1 + through the line before the first real section heading. + + ``empty`` flags a section whose slice is only its own heading line -- + two same-level headings with nothing between them -- so a caller can + skip summarizing content that doesn't exist rather than treating it + the same as a genuinely short section. """ if section_level is None: return [] line_count = len(lines) marks = [(text, line_start) for level, text, line_start in headings_with_lines if level == section_level] sections: List[Dict[str, Any]] = [] + if marks and marks[0][1] > 1 and any(line.strip() for line in lines[:marks[0][1] - 1]): + # Not "heading line + body": the whole span is body/title content, + # already confirmed non-blank above, so never flagged empty. + sections.append(_make_section(None, 1, marks[0][1] - 1, lines, empty=False)) for i, (text, line_start) in enumerate(marks): line_end = marks[i + 1][1] - 1 if i + 1 < len(marks) else line_count - hash_text = "\n".join(line.rstrip() for line in lines[line_start - 1:line_end]) - sections.append({ - "heading": text, - "line_start": line_start, - "line_end": line_end, - "hash": hashlib.sha256(hash_text.encode("utf-8")).hexdigest(), - "summary": None, - }) + # A real section's line_start is the heading line itself, so + # line_end <= line_start means no body lines followed it at all. + sections.append(_make_section(text, line_start, line_end, lines, empty=line_end <= line_start)) return sections + + +def _make_section( + heading: Optional[str], line_start: int, line_end: int, lines: List[str], *, empty: bool, +) -> Dict[str, Any]: + hash_text = "\n".join(line.rstrip() for line in lines[line_start - 1:line_end]) + return { + "heading": heading, + "line_start": line_start, + "line_end": line_end, + "hash": hashlib.sha256(hash_text.encode("utf-8")).hexdigest(), + "empty": empty, + "summary": None, + } # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-retrieval-sections +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-section-text +def section_text(lines: List[str], section: Dict[str, Any]) -> str: + """Slice a retrieval section's own raw text out of the file's lines. + + Shared by every consumer that needs a section's actual content rather + than just its boundaries (TF-IDF scoring, heading-nav search) -- one + implementation of the ``line_start``/``line_end`` slicing convention + instead of each consumer re-deriving it slightly differently. + """ + return "\n".join(lines[section["line_start"] - 1:section["line_end"]]) +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-section-text + + _MAX_READ_ATTEMPTS = 3 @@ -249,11 +289,13 @@ def build_doc_index(path: Path) -> Dict[str, Any]: sections: List[Dict[str, Any]] = [] for i, (level, text, line_start) in enumerate(headings): line_end = headings[i + 1][2] - 1 if i + 1 < len(headings) else line_count + hash_text = "\n".join(line.rstrip() for line in lines[line_start - 1:line_end]) sections.append({ "level": level, "heading": text, "line_start": line_start, "line_end": line_end, + "hash": hashlib.sha256(hash_text.encode("utf-8")).hexdigest(), "summary": None, }) @@ -299,15 +341,24 @@ def _read_cache_file(cache_path: Path) -> Optional[Dict[str, Any]]: def _has_schema_current_index(cached: Dict[str, Any]) -> bool: """``True`` only if ``cached`` carries every field a consumer - (``commands/doc_index.py``, :func:`annotate_section_summary`) reads by - subscript, at the schema version this module currently writes -- - treated the same as a stale/corrupt cache otherwise, so a partially - written, hand-edited, or pre-schema-bump cache triggers a clean rebuild - instead of a ``KeyError`` deep in a consumer. + (``commands/doc_index.py``, :func:`annotate_section_summary`, + :func:`diff_stale_sections`) reads by subscript, at the schema version + this module currently writes -- treated the same as a stale/corrupt + cache otherwise, so a partially written, hand-edited, or pre-schema + cache triggers a clean rebuild instead of a ``KeyError`` deep in a + consumer. Also checks every ``retrieval_sections`` and ``sections`` + entry carries a ``hash``: both fields were added after their + containers already existed, so a cache from one of those intermediate + schemas would otherwise pass the top-level field-presence check and + still raise on ``entry["hash"]``. """ if cached.get("schema_version") != _SCHEMA_VERSION: return False - return all(field in cached for field in _REQUIRED_INDEX_FIELDS) + if any(field not in cached for field in _REQUIRED_INDEX_FIELDS): + return False + if not all("hash" in section for section in cached["retrieval_sections"]): + return False + return all("hash" in section for section in cached["sections"]) def load_doc_index(path: Path) -> Optional[Dict[str, Any]]: @@ -347,15 +398,20 @@ def load_doc_index(path: Path) -> Optional[Dict[str, Any]]: if cached.get("etag") != current_etag: return None if not _has_schema_current_index(cached): - logger.debug("doc-index cache for %s is malformed or predates the current schema; rebuilding", path) + # A matching etag but a stale/malformed shape means a hand-edited + # or pre-schema-bump cache slipped past the etag check -- a real + # anomaly, not a routine miss, so warning rather than debug. + logger.warning("doc-index cache for %s is malformed or predates the current schema; rebuilding", path) return None return cached # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-load # @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-save -def save_doc_index(path: Path, index: Dict[str, Any]) -> None: - """Persist an index to its cache location. No-ops outside a Studio project. +def save_doc_index(path: Path, index: Dict[str, Any]) -> bool: + """Persist an index to its cache location. No-ops (returns ``False``) + outside a Studio project, so a caller can tell an actual write from a + silent no-op instead of assuming success unconditionally. Written atomically (temp file + ``os.replace``): a reader racing a concurrent writer sees either the old complete file or the new complete @@ -363,11 +419,9 @@ def save_doc_index(path: Path, index: Dict[str, Any]) -> None: """ cache_path = _index_cache_path(path) if cache_path is None: - return - cache_path.parent.mkdir(parents=True, exist_ok=True) - tmp_path = cache_path.with_name(f"{cache_path.name}.{os.getpid()}.tmp") - tmp_path.write_text(json.dumps(index, indent=2), encoding="utf-8") - os.replace(tmp_path, cache_path) + return False + atomic_write_text(cache_path, json.dumps(index, indent=2)) + return True # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-save @@ -444,25 +498,31 @@ def diff_stale_sections(path: Path) -> Optional[Dict[str, Any]]: Otherwise returns ``{"structural_change": bool, "unchanged": [...], "changed": [...]}``, where each entry is ``{"heading": str, "line_start": - int}`` -- the *current* (fresh) position, in document order. Sections - are matched by *position*, not heading text: duplicate heading titles - are real (see the ``toc-heading-duplicate`` check), so heading text - alone can't tell two same-named sections apart -- ``line_start`` is - what a caller should actually use to address "this specific section" - afterwards (e.g. to call :func:`annotate_section_summary`), with the - heading text included only for human-readable logging. When the - section *count* itself differs, ``structural_change`` is ``True`` and - ``changed``/``unchanged`` aren't populated -- a position-based diff - across a changed count can't be safely narrowed to "which ones - changed" without guessing, so the caller should fall back to a full - rebuild rather than have this function guess for it. + int}`` -- the *current* (fresh) position, in document order. Matched + primarily by *content hash*, not position: a section's hash appearing + in both the cached and fresh section lists is content that survived + unedited, however it moved, so a pure reorder with zero text changes + reports every section unchanged instead of misreporting the whole + document as edited. Matching is multiset-based (:class:`Counter`), so + genuine duplicate-content sections are paired up to the smaller of the + two counts, with only the surplus falling to ``changed`` -- correct + even when the same text legitimately appears more than once. Heading + text alone still can't identify a specific section (duplicate heading + titles are real -- see the ``toc-heading-duplicate`` check), so a + caller addressing "this specific section" afterwards (e.g. to call + :func:`annotate_section_summary`) uses the *current* ``line_start`` in + a returned entry, not a hash. When the section *count* itself differs, + ``structural_change`` is ``True`` and ``changed``/``unchanged`` aren't + populated -- a diff across a changed count can't be safely narrowed to + "which ones changed" without guessing, so the caller should fall back + to a full rebuild rather than have this function guess for it. """ cache_path = _index_cache_path(path) if cache_path is None or not cache_path.is_file(): return None cached = _read_cache_file(cache_path) - if cached is None or "retrieval_sections" not in cached: + if cached is None or not _has_schema_current_index(cached): return None fresh_sections = _compute_fresh_retrieval_sections(path) @@ -477,47 +537,40 @@ def diff_stale_sections(path: Path) -> Optional[Dict[str, Any]]: "changed": [_position_entry(s) for s in fresh_sections], } + remaining_old_hashes = Counter(s["hash"] for s in old_sections) unchanged: List[Dict[str, Any]] = [] changed: List[Dict[str, Any]] = [] - for old, new in zip(old_sections, fresh_sections, strict=True): - (unchanged if old["hash"] == new["hash"] else changed).append(_position_entry(new)) + for new in fresh_sections: + if remaining_old_hashes[new["hash"]] > 0: + remaining_old_hashes[new["hash"]] -= 1 + unchanged.append(_position_entry(new)) + else: + changed.append(_position_entry(new)) return {"structural_change": False, "unchanged": unchanged, "changed": changed} # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-diff-stale -def _with_cache_lock(cache_path: Path, fn): - """Run ``fn()`` -- a read-modify-write cycle against ``cache_path`` -- - under an exclusive lock on a sibling ``.lock`` file, serializing - concurrent callers so two overlapping read-modify-write cycles (e.g. - two :func:`annotate_section_summary` calls for different sections of - the same document, running from separate processes) can't each load - the same base index, mutate their own part, and have whichever writes - last silently discard the other's update. Mirrors - :func:`studio.utils.decision_log._append_locked`'s exact fallback: an - exclusive ``fcntl`` lock where available (POSIX), otherwise runs - ``fn()`` unlocked on platforms without it (e.g. Windows) -- atomicity - of each individual write is already guaranteed by :func:`save_doc_index` - regardless; only the cross-call serialization is best-effort there. - """ - try: - import fcntl # pylint: disable=import-outside-toplevel - except ImportError: - return fn() - lock_path = cache_path.with_name(f"{cache_path.name}.lock") - lock_path.parent.mkdir(parents=True, exist_ok=True) - with open(lock_path, "a", encoding="utf-8") as lock_fh: - fcntl.flock(lock_fh.fileno(), fcntl.LOCK_EX) - return fn() - # @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-annotate -def annotate_section_summary(path: Path, line_start: int, summary: str) -> bool: +def annotate_section_summary(path: Path, line_start: int, expected_hash: str, summary: str) -> bool: """Attach a one-line summary to a cached section, keyed by its line_start. Summaries are written by an LLM caller during a one-time enrichment pass, never generated inside this module. Returns ``False`` when no - valid (non-stale) cached index exists or no section matches - ``line_start`` -- callers should build the index first. + valid (non-stale) cached index exists, no section matches + ``line_start``, or ``expected_hash`` doesn't match that section's + current hash -- callers should build the index first, and re-resolve + on a hash mismatch rather than retry blindly. + + ``expected_hash`` must be the hash of the ``sections`` entry the + caller actually read and summarized (from a prior + :func:`get_or_build_doc_index`/:func:`build_doc_index` call). Without + this check, a document edited between that read and this write can + shift a *different* section into the same ``line_start`` (e.g. content + inserted above it), and matching by position alone would silently + attach one section's summary to another section's content -- a + caller can't tell the difference from the return value alone unless + the write is rejected outright. Updates the matching entry in both ``sections`` (any heading level) and ``retrieval_sections`` (the coarser grouping) when both have a section @@ -526,12 +579,16 @@ def annotate_section_summary(path: Path, line_start: int, summary: str) -> bool: list. A ``line_start`` that only matches ``sections`` (an off-level heading that isn't itself a retrieval section's start) updates only that list, which is correct: there is no corresponding retrieval - section to update. + section to update. The retrieval_sections match is only reached once + the ``sections``-level hash check above has already confirmed this + document position still holds the content the caller expects, so it + doesn't need (and can't reuse -- its hash covers a different span) a + second hash check of its own. The read-modify-write cycle (load, mutate one section, save) runs - under :func:`_with_cache_lock`, so two concurrent calls annotating - different sections of the same document don't race and silently drop - one side's update. + under :func:`studio.utils.atomic_io.with_file_lock`, so two concurrent + calls annotating different sections of the same document don't race + and silently drop one side's update. """ cache_path = _index_cache_path(path) if cache_path is None: @@ -542,22 +599,23 @@ def _read_modify_write() -> bool: if index is None: return False - matched = False + matched_section = None for section in index["sections"]: if section["line_start"] == line_start: - section["summary"] = summary - matched = True + matched_section = section break - if not matched: + if matched_section is None: + return False + if matched_section["hash"] != expected_hash: return False + matched_section["summary"] = summary for retrieval_section in index.get("retrieval_sections", []): if retrieval_section["line_start"] == line_start: retrieval_section["summary"] = summary break - save_doc_index(path, index) - return True + return save_doc_index(path, index) - return _with_cache_lock(cache_path, _read_modify_write) + return with_file_lock(cache_path.with_name(f"{cache_path.name}.lock"), _read_modify_write) # @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-annotate diff --git a/skills/studio/scripts/studio/utils/heading_nav.py b/skills/studio/scripts/studio/utils/heading_nav.py new file mode 100644 index 00000000..5fb3d134 --- /dev/null +++ b/skills/studio/scripts/studio/utils/heading_nav.py @@ -0,0 +1,95 @@ +"""Heading-nav retrieval over a document's retrieval sections. + +Purely mechanical, no LLM call: a case-insensitive literal-substring search +of a query against each retrieval section's own raw text -- the same +"``grep`` the query's words, then read the enclosing section" mechanism a +real ``grep -i`` invocation performs, mirrored here so it shares the same +section boundaries (:func:`studio.utils.doc_index.get_or_build_doc_index`) +every other retrieval method uses instead of re-deriving them. + +See constructorfabric/studio#104. + +@cpt-algo:cpt-studio-algo-traceability-validation-heading-nav:p1 +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any, Dict, List, Optional, Tuple + +from .doc_index import get_or_build_doc_index, section_text +from .toc import _fence_update + + +# @cpt-begin:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-strip-fences +def _strip_fenced_code_lines(lines: List[str]) -> List[str]: + """Blank out every line inside (or opening/closing) a fenced code block, + so a query match inside a code sample (a command, a variable name) isn't + counted the same as a genuine prose hit. Reuses ``toc.py``'s own fence- + tracking rather than a second implementation of "what counts as a fence" + that could silently drift from it. + """ + result: List[str] = [] + fence: Optional[Tuple[str, int]] = None + for line in lines: + new_fence = _fence_update(line, fence) + blank = new_fence != fence or fence is not None + fence = new_fence + result.append("" if blank else line) + return result +# @cpt-end:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-strip-fences + + +# @cpt-begin:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-search +def find_sections(path: Path, query: str) -> Dict[str, Any]: + """Find every retrieval section in ``path`` containing ``query`` literally. + + Case-insensitive substring match of the whole query string against each + section's own text -- deliberately not tokenized or word-split, since + this mirrors ``grep -i ""`` against the raw content, not a + ranking. A query that doesn't appear verbatim (different wording than + the source) has zero hits everywhere: this method has no semantic + fallback, by design -- that hard-failure mode is itself a real, useful + signal for a caller deciding whether to escalate past it. + + Returns ``{"matches": [...], "first_match": {...} | None}``: + + - ``matches``: every section with at least one hit, in document order, + each ``{"heading", "line_start", "line_end", "hit_count"}``. + - ``first_match``: ``matches[0]``, or ``None`` when nothing matched -- + the "grep's first hit -> enclosing section" pick this method's real + mechanism performs. + + An empty query, or a headingless document (no retrieval sections at + all), returns ``{"matches": [], "first_match": None}``. + + A hit inside a fenced code block (a command, a variable name in a + sample) doesn't count -- code blocks are excluded before counting, the + same fence-tracking ``toc.py`` uses elsewhere, so an incidental code- + sample match can't make this method pick a section on nothing but a + coincidental identifier. + """ + if not query.strip(): + return {"matches": [], "first_match": None} + + index = get_or_build_doc_index(path) + sections = index["retrieval_sections"] + if not sections: + return {"matches": [], "first_match": None} + + lines = _strip_fenced_code_lines(path.resolve().read_text(encoding="utf-8").split("\n")) + query_lower = query.lower() + + matches = [] + for section in sections: + hit_count = section_text(lines, section).lower().count(query_lower) + if hit_count: + matches.append({ + "heading": section["heading"], + "line_start": section["line_start"], + "line_end": section["line_end"], + "hit_count": hit_count, + }) + + return {"matches": matches, "first_match": matches[0] if matches else None} +# @cpt-end:cpt-studio-algo-traceability-validation-heading-nav:p1:inst-heading-nav-search diff --git a/skills/studio/scripts/studio/utils/okf.py b/skills/studio/scripts/studio/utils/okf.py new file mode 100644 index 00000000..3cf1dd31 --- /dev/null +++ b/skills/studio/scripts/studio/utils/okf.py @@ -0,0 +1,531 @@ +"""OKF (hierarchical summary) bundle: local, regenerable concept files and an +index for JIT retrieval's semantic fallback. + +Deterministic infrastructure only, matching this module's siblings +(``doc_index.py``, ``tfidf.py``): no LLM call happens here. Writing an +actual section summary is an external caller's job (an agent, dispatched +outside this codebase) -- this module tracks which concept files should +exist, detects when one is stale relative to its source section, and +persists whatever the caller writes. + +The whole bundle is local-only and gitignored (``.cache/okf/`` -- see +``.gitignore``): unlike the *content* of a summary, which is expensive to +regenerate (real LLM tokens), the fact that the bundle isn't checked in +just means a fresh clone rebuilds it from scratch the same way +``doc_index.py``'s own cache does. Nothing about this module assumes the +bundle survives across clones; it assumes only that it survives across +calls on the same machine, which is what makes the "only re-summarize what +changed" property of :func:`studio.utils.doc_index.diff_stale_sections` +actually save something. + +See constructorfabric/studio#104. + +@cpt-algo:cpt-studio-algo-traceability-validation-okf:p1 +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import re +import time +from collections import defaultdict +from pathlib import Path +from typing import Any, Dict, List, Optional, Set + +from .atomic_io import atomic_write_text, with_file_lock +from .doc_index import get_or_build_doc_index + +logger = logging.getLogger(__name__) + +_CACHE_SUBDIR = ".cache" +_BUNDLE_SUBDIR = "okf" +_MANIFEST_NAME = "manifest.json" +_INDEX_NAME = "index.md" + +_SLUG_RE = re.compile(r"[^a-z0-9]+") + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-bundle-dir +def _okf_bundle_dir(path: Path) -> Optional[Path]: + """Resolve ``/.cache/okf//`` for a source file. + + Same shape as ``doc_index._index_cache_path``: resolved from ``path`` + itself, not the process's working directory, so a bundle for a file + outside the caller's cwd still resolves the Studio directory that + actually owns it. Returns ``None`` outside a Studio-adapted project -- + callers should treat OKF as unavailable, not fail. + """ + from .files import find_studio_directory + + try: + studio_dir = find_studio_directory(path.resolve().parent) + except OSError as exc: + # A file whose parent can't be stat'd (permissions, a race) is not a + # reason to fail the caller -- just an unavailable bundle, like "no + # Studio directory found". Warning, not debug: this is a genuine + # anomaly, mirroring doc_index._index_cache_path's identical check. + logger.warning("okf bundle dir lookup failed for %s: %s", path, exc) + studio_dir = None + if studio_dir is None: + return None + + slug = hashlib.sha256(str(path.resolve()).encode("utf-8")).hexdigest()[:16] + return studio_dir / _CACHE_SUBDIR / _BUNDLE_SUBDIR / slug +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-bundle-dir + + +def _slugify(heading: Optional[str]) -> str: + """Kebab-case a heading for a concept-file name. Collisions between two + headings that slugify identically (e.g. duplicate titles, or titles + differing only in punctuation) are resolved by the caller prefixing + each filename with the section's document position, which is already + guaranteed unique -- this function doesn't need to be collision-free on + its own. ``None`` (the synthetic preamble section -- content before a + document's first real heading, see doc_index.py's + ``_build_retrieval_sections``) slugifies to a fixed, readable label + rather than crashing on a heading that was never a real string.""" + if heading is None: + return "preamble" + slug = _SLUG_RE.sub("-", heading.strip().lower()).strip("-") + return slug or "section" + + +def _concept_filename(position: int, heading: Optional[str]) -> str: + return f"{position:02d}-{_slugify(heading)}.md" + + +def _allocate_concept_filename(position: int, heading: Optional[str], manifest: Dict[str, Any]) -> str: + """Choose a concept filename for a genuinely new (never-before-written) + section, guaranteed not to collide with any filename already recorded + in the manifest. A naive position/heading-derived name alone could + otherwise coincide with a different, already-written section's own + file (e.g. duplicate headings after a reorder), letting + :func:`write_concept_file` silently overwrite that section's summary. + """ + existing = {e["concept_file"] for e in manifest.get("entries", [])} + base = _concept_filename(position, heading) + if base not in existing: + return base + stem, _, ext = base.rpartition(".") + suffix = 2 + while f"{stem}-{suffix}.{ext}" in existing: + suffix += 1 + return f"{stem}-{suffix}.{ext}" + + +_REQUIRED_MANIFEST_ENTRY_FIELDS = ("line_start", "concept_file", "built_from_hash") + + +def _is_valid_manifest_entry(entry: Any) -> bool: + """``True`` only if *entry* has every required field, of the type every + reader assumes. Presence alone isn't enough: ``line_start`` is used as + a dict key (``by_line_start``/hash-pool matching in + :func:`get_okf_status`) -- an unhashable value there (a list, a dict) + raises ``TypeError`` before this module's own malformed-manifest + fallback ever gets a chance to apply. + """ + if not isinstance(entry, dict) or not all(field in entry for field in _REQUIRED_MANIFEST_ENTRY_FIELDS): + return False + return ( + isinstance(entry["line_start"], int) and not isinstance(entry["line_start"], bool) + and isinstance(entry["concept_file"], str) + and isinstance(entry["built_from_hash"], str) + ) + + +def _is_valid_manifest_shape(manifest: Any) -> bool: + """``True`` only if *manifest* has the shape every reader assumes: a + dict with an ``entries`` list, each entry a dict carrying every field + :func:`get_okf_status`/:func:`write_concept_file` dereference by key, of + the type each is actually used as. A hand-edited or partially-written + manifest missing or mistyping one of these would otherwise surface as + an unhandled ``KeyError``/``TypeError`` deep inside a reader, instead of + the clean "treat this bundle as absent, rebuild" fallback every other + malformed-cache case in this codebase already gets. + """ + if not isinstance(manifest, dict): + return False + entries = manifest.get("entries") + if not isinstance(entries, list): + return False + return all(_is_valid_manifest_entry(entry) for entry in entries) + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-manifest-io +def load_okf_manifest(path: Path) -> Optional[Dict[str, Any]]: + """Load the OKF bundle manifest for ``path``, or ``None`` if absent/corrupt/unavailable.""" + bundle_dir = _okf_bundle_dir(path) + if bundle_dir is None: + return None + manifest_path = bundle_dir / _MANIFEST_NAME + if not manifest_path.is_file(): + return None + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError) as exc: + # Reached only once the caller has already confirmed the manifest + # file exists, so a failure here is real corruption or a + # permissions problem, not a routine miss -- warning, not debug, + # mirroring doc_index._read_cache_file's identical check. + logger.warning("okf manifest unreadable for %s: %s", path, exc) + return None + if not _is_valid_manifest_shape(manifest): + logger.warning("okf manifest for %s has an invalid/incomplete shape; treating as absent", path) + return None + return manifest + + +def save_okf_manifest(path: Path, manifest: Dict[str, Any]) -> bool: + """Persist the OKF bundle manifest atomically. No-ops (returns + ``False``) outside a Studio project. + + Atomic (temp file + ``os.replace``) so a crash mid-write leaves the + previous valid manifest in place instead of a torn/corrupt file -- + without this, a corrupt manifest is treated as "no manifest" by + :func:`load_okf_manifest`, collapsing every previously-current + section's status back to "missing" over a single interrupted write to + one section. + """ + bundle_dir = _okf_bundle_dir(path) + if bundle_dir is None: + return False + atomic_write_text(bundle_dir / _MANIFEST_NAME, json.dumps(manifest, indent=2)) + return True +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-manifest-io + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-status +def get_okf_status(path: Path) -> Dict[str, Any]: + """Report the OKF bundle's state against the document's *current* + retrieval sections -- not the manifest's own idea of what once existed. + + Returns ``{"available": bool, "bundle_dir": str | None, "entries": + [...]}`` . Each entry is ``{"heading", "line_start", "line_end", + "concept_file", "status"}``, where ``status`` is: + + - ``"missing"`` -- no manifest entry exists for this section yet (never + summarized, or a structural change added it since the last summary + pass -- see :func:`studio.utils.doc_index.diff_stale_sections`), or a + manifest entry exists but its concept file was deleted, truncated, or + corrupted out from under it (a manual edit, a crash mid-write outside + this module's own atomic write path) -- the manifest's hash alone + doesn't prove the file it points at still exists or holds real + content (see :func:`_concept_file_is_valid`). + - ``"stale"`` -- a manifest entry exists, but its recorded + ``built_from_hash`` no longer matches the section's current hash + (the source changed since the summary was written). + - ``"current"`` -- the manifest's recorded hash matches; the concept + file is trustworthy as-is. + + A section is matched to a manifest entry primarily by content hash, not + position: if inserting or reordering *other* sections shifted this + one's line numbers without touching its own text, its hash is + unchanged, and it's matched to the entry that recorded that hash + wherever that entry's own ``line_start`` now points -- preserving that + entry's stored ``concept_file`` rather than recomputing one from the + new position, so a reorder never claims a file that was never written. + Entries are consumed one at a time per hash (ties broken toward the + entry already at this exact ``line_start``, for stable pairing when + duplicate-content sections exist), so each moved section claims a + distinct entry rather than all collapsing onto the first. Only when no + entry's hash matches does an entry already recorded at this exact + ``line_start`` mark the section ``"stale"`` instead of ``"missing"``. + + ``available`` is ``False`` when there's no Studio directory to hold a + bundle at all (outside a Studio-adapted project) -- distinct from an + empty/all-missing bundle inside one. + """ + bundle_dir = _okf_bundle_dir(path) + if bundle_dir is None: + return {"available": False, "bundle_dir": None, "entries": []} + + index = get_or_build_doc_index(path) + sections = index["retrieval_sections"] + manifest_entries = (load_okf_manifest(path) or {"entries": []}).get("entries", []) + by_line_start = {entry["line_start"]: entry for entry in manifest_entries} + pool: Dict[str, List[Dict[str, Any]]] = defaultdict(list) + for entry in manifest_entries: + pool[entry["built_from_hash"]].append(entry) + + # Hash matching runs to completion for every section *before* any + # stale/missing fallback lookup -- interleaving them would let a + # section that moves *away* from a line_start (still unconsumed in + # `by_line_start` at that point in document order) leak its entry to + # a completely different, newly-added section that later happens to + # occupy that same old line_start. + matched_by_position = _match_sections_by_hash(sections, pool) + consumed_ids = {id(entry) for entry in matched_by_position.values()} + + entries = [ + _resolve_section_status( + section, position, matched_by_position.get(position), by_line_start, consumed_ids, bundle_dir, + ) + for position, section in enumerate(sections, start=1) + ] + return {"available": True, "bundle_dir": str(bundle_dir), "entries": entries} + + +def _match_sections_by_hash( + sections: List[Dict[str, Any]], pool: Dict[str, List[Dict[str, Any]]], +) -> Dict[int, Dict[str, Any]]: + """Pass 1 of :func:`get_okf_status`'s matching: resolve every section's + hash match (consuming ``pool`` as it goes) before any section's stale + fallback runs, so consumption never depends on document-order timing + between a moved section and whatever unrelated section now occupies + its old line_start. Returns matched entries keyed by position (1-based). + """ + matched_by_position: Dict[int, Dict[str, Any]] = {} + for position, section in enumerate(sections, start=1): + candidates = pool.get(section["hash"]) + if not candidates: + continue + same_slot = next( + (c for c in candidates if c["line_start"] == section["line_start"]), + candidates[0], + ) + candidates.remove(same_slot) + matched_by_position[position] = same_slot + return matched_by_position + + +def _concept_file_is_valid(concept_path: Path) -> bool: + """Minimal content-validity check for a concept file already confirmed + to exist on disk: real content always opens with the YAML frontmatter + block :func:`_build_frontmatter` writes. A physical-presence check + alone (``is_file()``) can't tell a genuine concept file from one + truncated, emptied, or corrupted after the fact -- this catches that + without needing full YAML parsing, which is more than this check needs + to answer "is there real content here at all". + """ + try: + content = concept_path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as exc: + logger.debug("okf concept file unreadable at %s: %s", concept_path, exc) + return False + return content.startswith("---\n") + + +def _resolve_section_status( + section: Dict[str, Any], + position: int, + matched_entry: Optional[Dict[str, Any]], + by_line_start: Dict[int, Dict[str, Any]], + consumed_ids: Set[int], + bundle_dir: Path, +) -> Dict[str, Any]: + """Resolve one retrieval section's OKF entry -- the per-section half of + :func:`get_okf_status`'s hash-primary matching, extracted so that + function's own local-variable count doesn't grow with each new + matching rule.""" + if matched_entry is not None: + concept_file = matched_entry["concept_file"] + status = "current" if _concept_file_is_valid(bundle_dir / concept_file) else "missing" + else: + stale_entry = by_line_start.get(section["line_start"]) + # An entry already claimed by a different section during hash + # matching (id() tracked in consumed_ids) "belongs" to whichever + # section moved away with it, not to whatever unrelated section + # now sits at its old line_start -- reusing it here would link + # this section to a concept file that was actually written for + # the moved one. + if stale_entry is not None and id(stale_entry) not in consumed_ids: + # The section at this line_start was actually summarized before + # (e.g. a heading rename with the body otherwise untouched) -- + # its real, already-written concept_file, not a filename + # freshly derived from the *current* heading/position that was + # never actually written to disk. + concept_file = stale_entry["concept_file"] + status = "stale" if _concept_file_is_valid(bundle_dir / concept_file) else "missing" + else: + concept_file = _concept_filename(position, section["heading"]) + status = "missing" + return { + "heading": section["heading"], + "line_start": section["line_start"], + "line_end": section["line_end"], + "concept_file": concept_file, + "status": status, + } +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-status + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-yaml-quote +def _yaml_quote(value: str) -> str: + """Render ``value`` as a YAML double-quoted scalar, safe against + embedded colons, quotes, backslashes, or newlines. Unescaped + interpolation would let any of those turn a frontmatter value into + invalid YAML, or -- for an embedded ``\\n---\\n`` -- prematurely close + the frontmatter block and let the rest of the value inject new + top-level keys. ``description`` is external-caller-supplied content + (an LLM's own summary text), so it can't be assumed free of any of + these. + """ + escaped = value.replace("\\", "\\\\").replace('"', '\\"') + escaped = escaped.replace("\n", "\\n").replace("\r", "\\r").replace("\t", "\\t") + return f'"{escaped}"' +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-yaml-quote + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-render-index +def _render_index_md( + source_path: Path, + status_entries: List[Dict[str, Any]], + descriptions_by_concept_file: Dict[str, str], +) -> str: + """Deterministic template, not an LLM call: the same bullet-list-of- + files-with-descriptions shape as the real OKF bundle this design was + validated against (``experiments/okf-full-166-pages/index.md``). + + Takes :func:`get_okf_status`'s own status entries -- the single + authoritative source for missing/stale/current -- rather than the raw + manifest, so this listing can't disagree with what ``cfs okf-status`` + reports: every current retrieval section appears (not just ones ever + written), a section whose concept file was deleted out from under it + shows as missing rather than a dead link, and a stale entry is + visibly marked rather than rendered identically to a current one. + + ``descriptions_by_concept_file`` is keyed by ``concept_file``, not + ``line_start``: a section resolved by content hash after a move keeps + its *original* manifest entry's ``concept_file`` (see + :func:`_resolve_section_status`), but reports its *current* line_start + -- keying by the current line_start would miss that entry's + description entirely and render "(no summary yet)" for a section that + genuinely has one. + """ + lines = [ + f"# OKF Bundle — {source_path.name}", + "", + f"Local, regenerable bundle for `{source_path}`. Not committed -- see `.gitignore`.", + "", + ] + for entry in status_entries: + heading = entry["heading"] if entry["heading"] is not None else "(preamble)" + if entry["status"] == "missing": + lines.append(f"* {heading} - not yet summarized") + continue + description = descriptions_by_concept_file.get(entry["concept_file"]) or "(no summary yet)" + marker = " _(stale -- source changed since written)_" if entry["status"] == "stale" else "" + lines.append(f"* [{heading}]({entry['concept_file']}) - {description}{marker}") + lines.append("") + return "\n".join(lines) +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-render-index + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-build-frontmatter +def _build_frontmatter(matched: Dict[str, Any], source_path: str, description: str, generated_by: str) -> str: + """Build a concept file's YAML frontmatter block, every value safely + quoted (see :func:`_yaml_quote`).""" + title = matched["heading"] if matched["heading"] is not None else "(preamble)" + resource = f"{source_path}#L{matched['line_start']}-L{matched['line_end']}" + generated_at = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) + return ( + "---\n" + f"title: {_yaml_quote(title)}\n" + f"description: {_yaml_quote(description)}\n" + f"resource: {_yaml_quote(resource)}\n" + f"generated: {{ by: {_yaml_quote(generated_by)}, at: {_yaml_quote(generated_at)} }}\n" + "---\n\n" + ) +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-build-frontmatter + + +# @cpt-begin:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-write-concept +def write_concept_file( + path: Path, + line_start: int, + *, + description: str, + body: str, + generated_by: str = "unknown", +) -> bool: + """Write (or overwrite) one section's concept file and refresh the index. + + This is the external-caller hook -- called by an agent after it has + actually produced a summary, never generated inside this module (same + role as :func:`studio.utils.doc_index.annotate_section_summary`, one + layer up). Returns ``False`` when ``line_start`` doesn't match a + *current* retrieval section (the caller should re-check + :func:`get_okf_status` -- the document may have changed structurally + since it was queried) or when there's no Studio directory to hold a + bundle in. + + Records the section's *current* hash as ``built_from_hash`` in the + manifest -- this is what lets :func:`get_okf_status` later tell + "current" from "stale" without re-reading the summary itself. + + The manifest's read-modify-write cycle, the concept-file write, and + the index.md regeneration all run under one exclusive lock (mirroring + :func:`studio.utils.doc_index.annotate_section_summary`'s own use of + the same primitive), so two concurrent calls writing different + sections of the same document's bundle can't each load the same base + manifest and have whichever saves last silently discard the other's + entry. Both file writes are atomic (temp file + ``os.replace``), so a + crash mid-write leaves the previous valid file in place instead of a + torn one. + """ + bundle_dir = _okf_bundle_dir(path) + if bundle_dir is None: + return False + + index = get_or_build_doc_index(path) + sections = index["retrieval_sections"] + matched = next((s for s in sections if s["line_start"] == line_start), None) + if matched is None: + return False + + position = sections.index(matched) + 1 + frontmatter = _build_frontmatter(matched, index["path"], description, generated_by) + + def _read_modify_write() -> bool: + manifest = load_okf_manifest(path) or {"source_path": index["path"], "entries": []} + manifest_entries = manifest.get("entries", []) + + # Reuse the resolved manifest entry's own concept_file when one + # already exists for this section's identity (matched by content + # hash, so a reorder/rename resolves to the same entry it always + # has) -- a filename freshly derived from just this section's + # *current* position/heading could otherwise collide with a + # different, already-written section's own file after a reorder, + # letting this write silently replace that section's summary. + current_status = get_okf_status(path) + existing_entry = next( + (e for e in current_status["entries"] if e["line_start"] == line_start and e["status"] != "missing"), + None, + ) + concept_filename = ( + existing_entry["concept_file"] if existing_entry is not None + else _allocate_concept_filename(position, matched["heading"], manifest) + ) + atomic_write_text(bundle_dir / concept_filename, frontmatter + body) + + new_entry = { + "heading": matched["heading"], + "line_start": line_start, + "concept_file": concept_filename, + "description": description, + "built_from_hash": matched["hash"], + } + # Replace by concept_file identity, not by raw line_start: a + # section resolved via hash match keeps its concept_file across a + # move even though the manifest's own recorded line_start for it + # is now stale. Keying the update by *this write's* line_start + # alone would silently orphan that entry whenever a different, + # newly-added section legitimately lands on that same line_start. + manifest["entries"] = sorted( + [e for e in manifest_entries if e["concept_file"] != concept_filename] + [new_entry], + key=lambda e: e["line_start"], + ) + manifest_saved = save_okf_manifest(path, manifest) + + status = get_okf_status(path) + descriptions_by_concept_file = {e["concept_file"]: e.get("description") for e in manifest["entries"]} + atomic_write_text( + bundle_dir / _INDEX_NAME, + _render_index_md(Path(index["path"]), status["entries"], descriptions_by_concept_file), + ) + return manifest_saved + + return with_file_lock(bundle_dir / f"{_MANIFEST_NAME}.lock", _read_modify_write) +# @cpt-end:cpt-studio-algo-traceability-validation-okf:p1:inst-okf-write-concept diff --git a/skills/studio/scripts/studio/utils/read_gate.py b/skills/studio/scripts/studio/utils/read_gate.py new file mode 100644 index 00000000..96bbdab3 --- /dev/null +++ b/skills/studio/scripts/studio/utils/read_gate.py @@ -0,0 +1,41 @@ +"""Large-read confirmation gate: a free line-count pre-check for a document +that's about to be read in full. + +Pure decision logic, no I/O -- deliberately not an interactive prompt. +This is a deterministic CLI, not the caller that actually reads a file and +answers a query; that caller (e.g. an agent) is the one positioned to ask a +human for confirmation. This module only produces the structured flag that +decision depends on, the same "boolean-in-the-result for an external caller +to act on" shape ``commands/chunk_input.py``'s ``plan_required`` already +uses for its own (differently-scoped) line-count threshold. + +@cpt-algo:cpt-studio-algo-traceability-validation-read-gate:p1 +""" + +from __future__ import annotations + +from typing import Any, Dict + +#: Real threshold measured this project's design session: the only read +#: among nine real candidate targets that crossed it was a whole-document +#: baseline read. Not a universal constant -- callers needing a different +#: threshold pass one explicitly. +DEFAULT_GATE_THRESHOLD_LINES = 5000 + + +# @cpt-begin:cpt-studio-algo-traceability-validation-read-gate:p1:inst-read-gate-check +def check_gate(total_lines: int, threshold: int = DEFAULT_GATE_THRESHOLD_LINES) -> Dict[str, Any]: + """Decide whether a read of ``total_lines`` should pause for confirmation. + + Returns ``{"needs_confirmation": bool, "total_lines": int, "threshold": + int}`` -- a structured verdict, not a side effect. A negative + ``total_lines`` (a caller's bug) is clamped to 0 rather than trusted, so + this can never claim confirmation is needed off of a nonsensical count. + """ + total_lines = max(0, total_lines) + return { + "needs_confirmation": total_lines > threshold, + "total_lines": total_lines, + "threshold": threshold, + } +# @cpt-end:cpt-studio-algo-traceability-validation-read-gate:p1:inst-read-gate-check diff --git a/skills/studio/scripts/studio/utils/tfidf.py b/skills/studio/scripts/studio/utils/tfidf.py new file mode 100644 index 00000000..8a02e201 --- /dev/null +++ b/skills/studio/scripts/studio/utils/tfidf.py @@ -0,0 +1,153 @@ +"""TF-IDF scoring over a document's retrieval sections. + +Purely mechanical, no LLM call: tokenize each retrieval section's text, +weight terms by rarity across the whole document, score a query as the sum +of term-frequency x inverse-document-frequency over the query's own terms. + +Scale assumption: this is JIT-retrieval infrastructure for a single real +document's worth of sections (the real corpus this was validated against +was a 166-page technical document, ~9 top-level sections) -- there's +deliberately no hard cap on section count or file size here, since any +concrete number would be an arbitrary guess with no real document behind +it, the same "real numbers, not assumptions" standard the rest of this +feature holds itself to. A caller feeding this an adversarially large or +malformed file is a resource-management concern for that caller, not +something this module should silently paper over with an unvalidated +threshold. + +See constructorfabric/studio#104. + +@cpt-algo:cpt-studio-algo-traceability-validation-tfidf:p1 +""" + +from __future__ import annotations + +import math +import re +from collections import Counter +from pathlib import Path +from typing import Any, Dict, List + +from .doc_index import get_or_build_doc_index, section_text + +_TOKEN_RE = re.compile(r"[a-z0-9]+") +_MIN_TOKEN_LENGTH = 3 + + +# @cpt-begin:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-tokenize +def tokenize(text: str) -> List[str]: + """Lowercase, alphanumeric-only tokens, dropping anything shorter than 3 + characters. Short tokens (``up``, ``is``, ``a``) are common-word noise + that dilutes real signal without adding any -- a real query tested + during this feature's design ("making up") lost its only distinguishing + word this way once ``up`` was filtered, reducing to the common word + ``making`` and producing a confidently wrong top-ranked section; that's + a real, documented failure mode of this scoring method, not a defect in + the filter itself. + """ + return [t for t in _TOKEN_RE.findall(text.lower()) if len(t) >= _MIN_TOKEN_LENGTH] +# @cpt-end:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-tokenize + + +# @cpt-begin:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-score-helpers +def _inverse_document_frequency(doc_tokens: List[List[str]]) -> Dict[str, float]: + """Standard idf: rarer terms (across this document's own sections) score + higher. ``+1`` keeps a term present in every section from scoring zero + weight rather than vanishing entirely.""" + section_count = len(doc_tokens) + document_frequency: Counter = Counter() + for tokens in doc_tokens: + for term in set(tokens): + document_frequency[term] += 1 + return { + term: math.log(section_count / (1 + count)) + 1 + for term, count in document_frequency.items() + } + + +def _rank_sections( + sections: List[Dict[str, Any]], + doc_tokens: List[List[str]], + query_terms: List[str], + idf: Dict[str, float], +) -> List[Dict[str, Any]]: + """Score each section as sum(tf(term, section) x idf(term)) over the + query's own terms, sorted descending.""" + ranked = [] + for section, tokens in zip(sections, doc_tokens, strict=True): + term_count = len(tokens) or 1 + term_frequency = Counter(tokens) + score = sum( + (term_frequency.get(term, 0) / term_count) * idf.get(term, 0.0) + for term in query_terms + ) + ranked.append({ + "heading": section["heading"], + "line_start": section["line_start"], + "line_end": section["line_end"], + "score": score, + }) + ranked.sort(key=lambda entry: entry["score"], reverse=True) + return ranked + + +def _confidence(ranked: List[Dict[str, Any]]) -> tuple: + """See :func:`score_sections` for what ``margin``/``unambiguous`` mean. + + A single section with a positive score is unambiguous by definition -- + there is nothing else it could be confused with, the same as a section + that beat every rival's zero score outright. + """ + if not ranked or ranked[0]["score"] <= 0: + return None, False + if len(ranked) == 1: + return None, True + second_score = ranked[1]["score"] + if not second_score: + return None, True + return ranked[0]["score"] / second_score, False +# @cpt-end:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-score-helpers + + +# @cpt-begin:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-score +def score_sections(path: Path, query: str) -> Dict[str, Any]: + """Score every retrieval section in ``path`` against ``query`` via TF-IDF. + + Reuses :func:`get_or_build_doc_index` for section boundaries (read once + per file, same as every other JIT-retrieval consumer), then reads the + file once more to tokenize each section's own text -- TF-IDF is + query-dependent, so unlike the structural index this can't be cached + across different queries. + + Returns ``{"ranked": [...], "margin": float | None, "unambiguous": + bool}``: + + - ``ranked``: sections sorted by score, descending, each + ``{"heading", "line_start", "line_end", "score"}``. + - ``margin``: ``top_score / second_score`` when both are positive and + finite; ``None`` when there are fewer than two sections, or the top + score itself is zero (no query term matched anywhere -- a margin + computed from zero would be meaningless, not just infinite). + - ``unambiguous``: ``True`` when the top score is positive and every + other section scores exactly zero -- the real, distinctive-term case + (a query like "KAPING" against a real document scored 0.0010 on its + one relevant section and 0.0000 everywhere else). Kept as an + explicit boolean rather than folding into ``margin`` as infinity, + since ``float("inf")`` doesn't round-trip through JSON. + + A headingless document (no retrieval sections at all) returns + ``{"ranked": [], "margin": None, "unambiguous": False}``. + """ + index = get_or_build_doc_index(path) + sections = index["retrieval_sections"] + if not sections: + return {"ranked": [], "margin": None, "unambiguous": False} + + lines = path.resolve().read_text(encoding="utf-8").split("\n") + doc_tokens = [tokenize(section_text(lines, section)) for section in sections] + idf = _inverse_document_frequency(doc_tokens) + ranked = _rank_sections(sections, doc_tokens, tokenize(query), idf) + margin, unambiguous = _confidence(ranked) + + return {"ranked": ranked, "margin": margin, "unambiguous": unambiguous} +# @cpt-end:cpt-studio-algo-traceability-validation-tfidf:p1:inst-tfidf-score diff --git a/skills/studio/scripts/studio/utils/toc.py b/skills/studio/scripts/studio/utils/toc.py index 03ecfdb9..91da255c 100644 --- a/skills/studio/scripts/studio/utils/toc.py +++ b/skills/studio/scripts/studio/utils/toc.py @@ -370,11 +370,28 @@ def add_toc_max_level_argument(parser: argparse.ArgumentParser) -> None: def _find_frontmatter_end(lines: List[str]) -> int: - """Return the index after YAML frontmatter when present.""" - if not lines or lines[0].strip() != "---": + """Return the index after YAML frontmatter when present. + + YAML allows a document to close with either ``---`` (a new document + marker, which this codebase treats as "end of frontmatter") or ``...`` + (the explicit end-of-document marker) -- accepting only ``---`` meant + frontmatter closed with ``...`` was never recognized as closed at all, + so every line after it (including every real heading) was treated as + still inside frontmatter and skipped entirely. + + Both the opening and closing delimiter must start at column zero: + ``rstrip()`` (trailing whitespace only), never ``strip()``. An indented + `` ---`` on the first line is valid Markdown as an indented thematic + break, not frontmatter at all; an indented ``---``/``...`` later on is + valid content inside a YAML block-scalar value (e.g. + ``description: |\\n ---``). Treating either as a real delimiter would + mis-scope frontmatter and let the rest of it parse as Markdown (a + stray ``# note`` becoming a heading). + """ + if not lines or lines[0].rstrip() != "---": return 0 idx = 1 - while idx < len(lines) and lines[idx].strip() != "---": + while idx < len(lines) and lines[idx].rstrip() not in ("---", "..."): idx += 1 if idx < len(lines): idx += 1 @@ -867,7 +884,14 @@ def _check_section_lengths( _DESCRIPTION_FIELD_RE = re.compile(r"^description\s*:\s*(.*)$") -_BLOCK_SCALAR_RE = re.compile(r"^[|>][+\-]?\d*$") +# YAML 1.2.2 allows the chomping (+/-) and indentation (1-9) indicators in +# either order, and a trailing "# comment" (preceded by whitespace) on the +# header line itself: |, |-, |2, |2-, |-2, | # comment. The original regex +# only matched one indicator order and rejected any trailing comment, so a +# real header like "|2-" or "| # TODO" fell through to the "has a real +# description" branch instead of being recognized as an (possibly empty) +# block scalar at all. +_BLOCK_SCALAR_RE = re.compile(r"^[|>](?:[+\-]?[1-9]?|[1-9][+\-]?)(?:\s+#.*)?$") def _quoted_value_is_empty(value: str) -> bool: @@ -907,7 +931,7 @@ def _frontmatter_has_description(lines: List[str], frontmatter_end: int) -> bool """ body = lines[1:frontmatter_end - 1] for i, line in enumerate(body): - match = _DESCRIPTION_FIELD_RE.match(line.strip()) + match = _DESCRIPTION_FIELD_RE.match(line) if not match: continue value = match.group(1).strip() diff --git a/skills/studio/scripts/studio/utils/ui.py b/skills/studio/scripts/studio/utils/ui.py index 491b8dbb..b9966304 100644 --- a/skills/studio/scripts/studio/utils/ui.py +++ b/skills/studio/scripts/studio/utils/ui.py @@ -20,10 +20,14 @@ ui.result(data_dict, human_fn=_format_init) """ +import argparse import json import os import sys -from typing import Any, Callable, Dict, List, Optional +from pathlib import Path +from typing import Any, Callable, Dict, List, Optional, Tuple, TypeVar + +T = TypeVar("T") # --------------------------------------------------------------------------- @@ -258,6 +262,153 @@ def result( # @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-result-human +# @cpt-begin:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-json-safe-argparse +class _ArgumentParsingError(Exception): + """Raised by :class:`JsonSafeArgumentParser` instead of the process + exiting directly, so :func:`parse_args_or_json_error` can turn a + parsing failure into the standard JSON/human ERROR result.""" + + +class JsonSafeArgumentParser(argparse.ArgumentParser): + """An ``ArgumentParser`` whose parsing *failures* raise instead of + printing a plain-text usage banner and calling ``sys.exit`` directly. + + A missing/malformed argument otherwise bypasses this project's own + ``--json`` output contract entirely: ``parser.error()`` writes + unparseable plain text to stderr and exits, never touching + :func:`result`, so a caller that always parses stdout as JSON gets an + empty payload and an unhandled stderr string instead of a structured + error. ``--help``/``--version`` are unaffected -- those call + ``exit()``, not ``error()``, and stay human-oriented on purpose. + """ + + def error(self, message: str) -> None: # noqa: D102 - argparse's own signature + raise _ArgumentParsingError(message) + + +def parse_args_or_json_error(parser: "JsonSafeArgumentParser", argv: List[str]) -> Optional[argparse.Namespace]: + """Parse ``argv`` with ``parser``, or emit the standard ERROR result and + return ``None`` on a parsing failure -- the caller should ``return 2``. + + ``--help``/``--version`` still exit the process directly and are not + caught here (see :class:`JsonSafeArgumentParser`). + """ + try: + return parser.parse_args(argv) + except _ArgumentParsingError as exc: + result( + {"status": "ERROR", "message": str(exc)}, + human_fn=lambda d: error(d["message"]), + ) + return None +# @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-json-safe-argparse + + +# @cpt-begin:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-require-existing-file +def require_existing_file(file_arg: str) -> Optional[Path]: + """Resolve a CLI file-path argument to an existing file, or emit the + standard "File not found" ERROR result (JSON or human, via :func:`result`) + and return ``None``. + + Shared by every command taking a single file-path argument (``doc-index``, + ``tfidf-score``, ``okf-status``, ...) -- previously each reimplemented the + same resolve-and-check block independently, which pylint's duplicate-code + check correctly caught once a third copy appeared. + """ + filepath = Path(file_arg).resolve() + if filepath.is_file(): + return filepath + result( + {"file": str(filepath), "status": "ERROR", "message": "File not found"}, + human_fn=lambda d: error(f"{d['file']}: {d['message']}"), + ) + return None +# @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-require-existing-file + + +# @cpt-begin:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-parse-file-command +def parse_file_command( + parser: JsonSafeArgumentParser, argv: List[str], *, file_attr: str = "file", +) -> "tuple[Optional[argparse.Namespace], Optional[Path]]": + """Parse ``argv``, then resolve+validate its ``file_attr`` positional as + an existing file -- the two-step dance every single-file-argument + command needs (safe argparse, then :func:`require_existing_file`), + extracted once a third command repeated it identically enough for + pylint's duplicate-code check to catch it (the same reason + :func:`require_existing_file` itself exists). + + Returns ``(args, filepath)``. ``filepath`` is ``None`` on either + failure (a parsing error or a missing/non-file path) -- the caller + should ``return 2`` in that case; the appropriate ERROR result has + already been emitted either way. ``args`` is also ``None`` specifically + on a parsing failure, since there's nothing parsed to return. + """ + args = parse_args_or_json_error(parser, argv) + if args is None: + return None, None + filepath = require_existing_file(getattr(args, file_attr)) + return args, filepath +# @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-parse-file-command + + +# @cpt-begin:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-report-read-error +def report_read_error(filepath: Path, exc: BaseException) -> None: + """Emit the standard "Cannot read file" ERROR result (JSON or human) for + an ``OSError``/``UnicodeDecodeError`` raised while reading an already- + existing file (a non-UTF-8 file, a permissions failure, a race after + the initial existence check). + + Shared by every command that reads a file's content after + :func:`parse_file_command` has already confirmed it exists (``doc-index``, + ``tfidf-score``, ...) -- extracted once a second copy of the identical + try/except/result block appeared, the same duplicate-code trigger + :func:`require_existing_file` and :func:`parse_file_command` were each + extracted for. + """ + result( + {"file": str(filepath), "status": "ERROR", "message": f"Cannot read file: {exc}"}, + human_fn=lambda d: error(f"{d['file']}: {d['message']}"), + ) +# @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-report-read-error + + +# @cpt-begin:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-call-with-read-error-handling +def call_with_read_error_handling(filepath: Path, fn: Callable[[], T]) -> Tuple[Optional[T], Optional[int]]: + """Call ``fn()`` -- a zero-arg callable that reads *filepath*'s content + -- catching ``OSError``/``UnicodeDecodeError`` and reporting via + :func:`report_read_error`. + + Returns ``(result, None)`` on success, or ``(None, 2)`` on a caught + read failure; callers should ``return exit_code`` when it isn't + ``None``. Extracted once the identical "call the thing that reads a + file, catch its two read-failure exceptions, report, return 2" block + appeared across four commands (``heading-nav``, ``retrieve``, + ``read-gate``, ``okf-status``) -- the same duplicate-code trigger + every other shared helper in this module was extracted for. + """ + try: + return fn(), None + except (OSError, UnicodeDecodeError) as exc: + report_read_error(filepath, exc) + return None, 2 +# @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-call-with-read-error-handling + + +# @cpt-begin:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-display-heading +def display_heading(heading: Optional[str]) -> str: + """Render a retrieval section's heading for human/text display. + + ``None`` is the synthetic preamble section's heading (content before a + document's first real heading -- see doc_index.py's + ``_build_retrieval_sections``), never a real heading's value; shown as + a readable label instead of the literal string "None". Shared by every + command that prints a section's heading (``doc-index``, ``tfidf-score``, + ``okf-status``, ...). + """ + return heading if heading is not None else "(preamble)" +# @cpt-end:cpt-studio-algo-core-infra-render-info-human:p1:inst-ui-display-heading + + # --------------------------------------------------------------------------- # Path helpers # --------------------------------------------------------------------------- @@ -292,6 +443,13 @@ class _UI: # pylint: disable=too-few-public-methods table = staticmethod(table) file_action = staticmethod(file_action) result = staticmethod(result) + require_existing_file = staticmethod(require_existing_file) + parse_file_command = staticmethod(parse_file_command) + report_read_error = staticmethod(report_read_error) + call_with_read_error_handling = staticmethod(call_with_read_error_handling) + display_heading = staticmethod(display_heading) + JsonSafeArgumentParser = JsonSafeArgumentParser + parse_args_or_json_error = staticmethod(parse_args_or_json_error) is_json = staticmethod(is_json_mode) relpath = staticmethod(relpath) diff --git a/tests/test_atomic_io.py b/tests/test_atomic_io.py new file mode 100644 index 00000000..94312192 --- /dev/null +++ b/tests/test_atomic_io.py @@ -0,0 +1,114 @@ +"""Tests for the shared atomic-write and file-locking primitives (atomic_io.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import builtins +from pathlib import Path + +import pytest + +from studio.utils.atomic_io import atomic_write_text, with_file_lock + + +class TestAtomicWriteText: + def test_writes_content(self, tmp_path: Path): + target = tmp_path / "sub" / "file.txt" + atomic_write_text(target, "hello") + assert target.read_text(encoding="utf-8") == "hello" + + def test_creates_parent_directories(self, tmp_path: Path): + target = tmp_path / "a" / "b" / "c.txt" + atomic_write_text(target, "x") + assert target.is_file() + + def test_overwrites_existing_content(self, tmp_path: Path): + target = tmp_path / "file.txt" + atomic_write_text(target, "first") + atomic_write_text(target, "second") + assert target.read_text(encoding="utf-8") == "second" + + def test_leaves_no_temp_file_behind(self, tmp_path: Path): + target = tmp_path / "file.txt" + atomic_write_text(target, "content") + names = [p.name for p in tmp_path.iterdir()] + assert names == ["file.txt"] + + def test_cleans_up_the_temp_file_when_replace_fails(self, tmp_path: Path, monkeypatch): + """A failure between the temp write and the final os.replace (disk + full, a permissions change mid-write) must not leave an orphaned + temp file behind, and must propagate the original error rather + than swallowing it.""" + import os as os_module + + target = tmp_path / "file.txt" + + def _raise_replace(*_a, **_k): + raise OSError("disk full") + + monkeypatch.setattr(os_module, "replace", _raise_replace) + with pytest.raises(OSError, match="disk full"): + atomic_write_text(target, "content") + + assert not target.exists() + assert list(tmp_path.iterdir()) == [] + + def test_concurrent_writes_to_the_same_target_do_not_collide(self, tmp_path: Path): + """CodeRabbit PR #110: a PID-based temp filename is shared by every + call within one process -- two threads writing the same target + could each pick up the other's temp file mid-write, causing a + FileNotFoundError on os.replace or one call silently publishing + the other's content. A unique temp name per call (tempfile.mkstemp) + closes that window: each thread's write completes cleanly, and the + final content is one call's payload in full, never a torn mix.""" + import threading + + target = tmp_path / "file.txt" + barrier = threading.Barrier(2) + errors = [] + payloads = ["a" * 200_000, "b" * 200_000] + + def _write(content): + barrier.wait() + try: + atomic_write_text(target, content) + except OSError as exc: + errors.append(exc) + + threads = [threading.Thread(target=_write, args=(p,)) for p in payloads] + for t in threads: + t.start() + for t in threads: + t.join() + + assert not errors + assert target.read_text(encoding="utf-8") in payloads + + +class TestWithFileLock: + def test_runs_and_returns_the_callback_result(self, tmp_path: Path): + lock_path = tmp_path / "x.lock" + assert with_file_lock(lock_path, lambda: 42) == 42 + + def test_creates_parent_directory_for_the_lock_file(self, tmp_path: Path): + lock_path = tmp_path / "nested" / "x.lock" + with_file_lock(lock_path, lambda: None) + assert lock_path.parent.is_dir() + + def test_runs_unlocked_when_fcntl_is_unavailable(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Mirrors decision_log.py's own test for the identical fallback + shape: a platform without fcntl (e.g. Windows) still runs the + callback, just without cross-process serialization.""" + real_import = builtins.__import__ + + def _no_fcntl(name, *args, **kwargs): + if name == "fcntl": + raise ImportError("fcntl unavailable on this platform") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", _no_fcntl) + lock_path = tmp_path / "x.lock" + assert with_file_lock(lock_path, lambda: "ran") == "ran" + assert not lock_path.exists() # never created -- the fallback never opens it diff --git a/tests/test_cascade.py b/tests/test_cascade.py new file mode 100644 index 00000000..80cfcf50 --- /dev/null +++ b/tests/test_cascade.py @@ -0,0 +1,352 @@ +"""Tests for the two-tier JIT-retrieval cascade (cascade.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from studio.commands.cascade import cmd_retrieve +from studio.utils.cascade import route_query, route_tier1, route_tier2 +from studio.utils.doc_index import get_or_build_doc_index +from studio.utils.okf import write_concept_file + +_SAMPLE = ( + "## Introduction\n\n" + "This section introduces the KAPING framework for knowledge graphs.\n\n" + "## Related Work\n\n" + "This section covers unrelated background material with no overlap.\n" +) + +# Same shape as findings.md's "zero-shot" adversarial test: heading-nav and +# TF-IDF agree on the same section (both pick SectionA -- three raw hits), +# but the margin is finite (not unambiguous), since the query term also +# appears once, diluted, in SectionB's much longer text. +_DIFFUSE_MARGIN_SAMPLE = ( + "## SectionA\n\nwidget widget widget banana.\n\n" + "## SectionB\n\n" + ("filler word text here. " * 40) + "widget mentioned once here.\n" +) + +# Heading-nav's first hit (SectionA, more raw occurrences) disagrees with TF-IDF's +# length-normalized top pick (SectionB, denser but shorter) -- same shape as +# findings.md's real LongMemEval split. +_DISAGREEMENT_SAMPLE = ( + "## SectionA\n\ngadget appears here. " + ("filler filler filler filler. " * 60) + "\n\n" + "## SectionB\n\ngadget gadget gadget.\n" +) + + +def _write(tmp_path: Path, content: str = _SAMPLE, name: str = "doc.md") -> Path: + f = tmp_path / name + f.write_text(content, encoding="utf-8") + return f + + +class TestRouteTier1: + def test_row1_heading_nav_no_hits_escalates(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = route_tier1(f, "making up") + assert result == {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + + def test_row2_agree_unambiguous_resolves_at_tier1(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = route_tier1(f, "KAPING") + assert result["tier"] == "resolved" + assert result["reason"] == "heading_nav_tfidf_agree_large_margin" + assert result["candidates"] == [{"heading": "Introduction", "line_start": 1, "line_end": 4}] + + def test_row3_disagreement_resolves_multi_with_both_candidates(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DISAGREEMENT_SAMPLE) + result = route_tier1(f, "gadget") + assert result["tier"] == "resolved_multi" + assert result["reason"] == "heading_nav_tfidf_disagree" + headings = {c["heading"] for c in result["candidates"]} + assert headings == {"SectionA", "SectionB"} + + def test_row4_agree_diffuse_margin_escalates(self, tmp_path: Path, monkeypatch): + """Real, reproduced shape of findings.md's "zero-shot" adversarial + test: heading-nav and TF-IDF agree on the same section, but the + margin is finite (not unambiguous) -- and that agreed pick is + documented as the wrong answer. Confirms the conservative default + (only unambiguous counts as a safe large margin) escalates here.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + result = route_tier1(f, "widget") + assert result["tier"] == "escalate" + assert result["reason"] == "diffuse_margin" + assert result["candidates"] == [{"heading": "SectionA", "line_start": 1, "line_end": 4}] + + def test_margin_threshold_can_enable_a_numeric_large_margin_resolution(self, tmp_path: Path, monkeypatch): + """The default (None) requires unambiguous; passing a numeric + threshold is an explicit opt-in to a less conservative policy.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + result = route_tier1(f, "widget", margin_threshold=1.0) + assert result["tier"] == "resolved" + assert result["reason"] == "heading_nav_tfidf_agree_large_margin" + + +class TestRouteTier2: + def test_no_bundle_at_all_recommends_baseline(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + tier1 = {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + result = route_tier2(f, tier1) + assert result == {"recommendation": "baseline", "reason": "no_current_okf_bundle"} + + def test_bundle_exists_but_only_missing_entries_is_treated_as_no_bundle(self, tmp_path: Path, monkeypatch): + """Real bug caught during manual verification: get_okf_status() + returns one entry per retrieval section even when nothing has ever + been summarized, all with status "missing" -- an available + bundle_dir with every entry missing means no concept file has + actually been written, which is "no bundle" for this decision.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + tier1 = route_tier1(f, "widget") + result = route_tier2(f, tier1) + assert result["recommendation"] == "baseline" + assert "okf_needs_rebuild" not in result + + def test_no_candidate_with_a_partially_summarized_bundle_falls_back_to_baseline( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #111: row 1 (heading-nav found zero hits) has no + candidate section to narrow to, so the external OKF file-selector + could land on any section in the bundle. Recommending OKF while + even one other section is stale/missing would let that external + step pick exactly the untrustworthy one.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + section_a = index["retrieval_sections"][0] + write_concept_file(f, section_a["line_start"], description="d", body="b") # SectionB left missing + + tier1 = {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + result = route_tier2(f, tier1) + assert result["recommendation"] == "baseline" + assert result["okf_needs_rebuild"] is True + + def test_no_candidate_with_a_fully_current_bundle_recommends_okf(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + for section in index["retrieval_sections"]: + write_concept_file(f, section["line_start"], description="d", body="b") + + tier1 = {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + result = route_tier2(f, tier1) + assert result["recommendation"] == "okf" + + def test_current_bundle_for_candidate_recommends_okf(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + section_a = index["retrieval_sections"][0] + write_concept_file(f, section_a["line_start"], description="d", body="b") + + tier1 = route_tier1(f, "widget") + result = route_tier2(f, tier1) + assert result["recommendation"] == "okf" + assert result["bundle_dir"] + + def test_stale_bundle_for_candidate_falls_back_to_baseline_with_rebuild_flag(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + section_a = index["retrieval_sections"][0] + write_concept_file(f, section_a["line_start"], description="d", body="b") + + f.write_text(_DIFFUSE_MARGIN_SAMPLE.replace("banana.", "banana banana."), encoding="utf-8") + tier1 = route_tier1(f, "widget") + result = route_tier2(f, tier1) + assert result["recommendation"] == "baseline" + assert result["okf_needs_rebuild"] is True + + def test_expected_future_queries_adds_break_even_math(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + tier1 = {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + result = route_tier2(f, tier1, expected_future_queries=20) + breakeven = result["build_okf_break_even"] + assert breakeven["okf_total_tokens"] == 301_187 + 45_735 * 20 + assert breakeven["baseline_total_tokens"] == 333_573 * 20 + assert breakeven["building_okf_would_pay_off"] is True + + def test_no_expected_future_queries_omits_break_even_math(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + tier1 = {"tier": "escalate", "reason": "heading_nav_no_hits", "candidates": []} + result = route_tier2(f, tier1) + assert "build_okf_break_even" not in result + + def test_candidate_that_no_longer_matches_any_section_falls_back_to_baseline( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #111: if the document changed structurally between + Tier 1 picking a candidate and this re-derived status (a real, + if narrow, race), the candidate's line_start may no longer match + any current section. Silently dropping it would leave `relevant` + empty, and `any(... for entry in [])` is vacuously False -- + recommending OKF on a candidate that was never actually verified.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + section_a = index["retrieval_sections"][0] + write_concept_file(f, section_a["line_start"], description="d", body="b") + + bogus_tier1 = { + "tier": "escalate", "reason": "diffuse_margin", + "candidates": [{"heading": "Ghost", "line_start": 99999, "line_end": 99999}], + } + result = route_tier2(f, bogus_tier1) + assert result["recommendation"] == "baseline" + assert result["okf_needs_rebuild"] is True + + +class TestRouteQuery: + def test_resolved_at_tier1_never_calls_tier2(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = route_query(f, "KAPING") + assert result["tier"] == "resolved" + assert "tier2" not in result + assert "read_gate" not in result + + def test_resolved_multi_never_calls_tier2(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DISAGREEMENT_SAMPLE) + result = route_query(f, "gadget") + assert result["tier"] == "resolved_multi" + assert "tier2" not in result + + def test_escalation_to_baseline_wires_in_the_read_gate(self, tmp_path: Path, monkeypatch): + """The integration point findings.md flagged as still-missing: when + Tier 2 recommends baseline, the read-gate check runs against the + real doc-index line count instead of leaving it disconnected.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## A\n\n" + "\n".join(f"line {i}" for i in range(20)) + "\n") + result = route_query(f, "making up") + assert result["tier"] == "escalate" + assert result["tier2"]["recommendation"] == "baseline" + assert result["read_gate"]["needs_confirmation"] is False + assert result["read_gate"]["total_lines"] == get_or_build_doc_index(f)["total_lines"] + + def test_escalation_to_okf_does_not_run_the_read_gate(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + section_a = index["retrieval_sections"][0] + write_concept_file(f, section_a["line_start"], description="d", body="b") + + result = route_query(f, "widget") + assert result["tier2"]["recommendation"] == "okf" + assert "read_gate" not in result + + +class TestCmdRetrieve: + def test_missing_file(self, tmp_path: Path, capsys): + rc = cmd_retrieve([str(tmp_path / "nope.md"), "query"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_missing_required_argument_emits_json_error_not_a_plain_text_banner(self, capsys): + """CodeRabbit PR #111: cmd_retrieve now uses JsonSafeArgumentParser + (like every other single-file command), so omitting a required + positional must still emit the project's own --json ERROR + contract, not argparse's default usage banner + SystemExit.""" + rc = cmd_retrieve([]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_margin_threshold_rejects_non_positive_values(self, capsys): + """CodeRabbit PR #111: a negative or zero --margin-threshold would + make the safety-relevant margin comparison fire on virtually any + result, defeating the cascade's own documented safety margin.""" + rc = cmd_retrieve(["doc.md", "query", "--margin-threshold", "-1"]) + assert rc == 2 + assert json.loads(capsys.readouterr().out)["status"] == "ERROR" + rc = cmd_retrieve(["doc.md", "query", "--margin-threshold", "0"]) + assert rc == 2 + assert json.loads(capsys.readouterr().out)["status"] == "ERROR" + + def test_margin_threshold_rejects_non_finite_values(self, capsys): + rc = cmd_retrieve(["doc.md", "query", "--margin-threshold", "nan"]) + assert rc == 2 + assert json.loads(capsys.readouterr().out)["status"] == "ERROR" + rc = cmd_retrieve(["doc.md", "query", "--margin-threshold", "inf"]) + assert rc == 2 + assert json.loads(capsys.readouterr().out)["status"] == "ERROR" + + def test_basic_json_output(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + rc = cmd_retrieve([str(f), "KAPING"]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["tier"] == "resolved" + + def test_margin_threshold_flag(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + rc = cmd_retrieve([str(f), "widget", "--margin-threshold", "1.0"]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["tier"] == "resolved" + + def test_human_output_escalation_with_read_gate(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## A\n\n" + "\n".join(f"line {i}" for i in range(6000)) + "\n") + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_retrieve([str(f), "making up"]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "tier 2 recommendation" in out + assert "needs confirmation" in out + + def test_human_output_resolved(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_retrieve([str(f), "KAPING"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "Introduction" in capsys.readouterr().out + + def test_human_output_okf_needs_rebuild(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, _DIFFUSE_MARGIN_SAMPLE) + index = get_or_build_doc_index(f) + section_a = index["retrieval_sections"][0] + write_concept_file(f, section_a["line_start"], description="d", body="b") + f.write_text(_DIFFUSE_MARGIN_SAMPLE.replace("banana.", "banana banana."), encoding="utf-8") + + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_retrieve([str(f), "widget"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "needs a rebuild" in capsys.readouterr().out diff --git a/tests/test_decision_log.py b/tests/test_decision_log.py index dc826070..ef541911 100644 --- a/tests/test_decision_log.py +++ b/tests/test_decision_log.py @@ -219,6 +219,65 @@ def test_record_invocation_shape(log_path: Path) -> None: assert ev["payload"]["args"] == {"paths": 1} # arg-shape summary, never raw argv +def test_record_read_shape(log_path: Path) -> None: + dl.record_read("tfidf", "doc.md", 8925, 49676, source="cli", path=log_path) + ev = next(iter(dl.read_events(log_path, event="read"))) + assert ev["payload"]["method"] == "tfidf" + assert ev["payload"]["lines"] == 8925 + assert ev["payload"]["tokens"] == 49676 + assert ev["payload"]["source"] == "cli" + + +def test_read_is_a_declared_event_name() -> None: + assert "read" in dl.EVENTS + + +def test_summarize_reads_aggregates_tokens_and_lines_per_method(log_path: Path) -> None: + dl.record_read("tfidf", "doc.md", 8925, 49676, path=log_path) + dl.record_read("tfidf", "doc.md", 8925, 12000, path=log_path) + dl.record_read("baseline", "doc.md", 8925, 333573, path=log_path) + dl.record("routing", {"a": 1}, path=log_path) # non-read event, must be ignored + + result = dl.summarize_reads(log_path) + assert result["methods"]["tfidf"] == {"count": 2, "total_tokens": 61676, "total_lines": 17850} + assert result["methods"]["baseline"] == {"count": 1, "total_tokens": 333573, "total_lines": 8925} + assert result["total_tokens"] == 61676 + 333573 + + +def test_summarize_reads_on_empty_log_returns_no_methods(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + assert dl.summarize_reads() == {"methods": {}, "total_tokens": 0} + + +def _append_raw_read_line(log_path: Path, payload) -> None: + log_path.parent.mkdir(parents=True, exist_ok=True) + obj = {"schema": dl.SCHEMA_VERSION, "ts": "2026-01-01T00:00:00+00:00", + "run_id": "x", "decision_id": "", "event": "read", "command": "", "payload": payload} + with log_path.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(obj) + "\n") + + +def test_summarize_reads_skips_a_non_dict_payload(log_path: Path) -> None: + """CodeRabbit PR #111: a parseable "read" record whose payload isn't a + dict (hand-edited or corrupted log) must not crash .get() -- skipped, + same tolerance read_events() already gives an unparseable line.""" + _append_raw_read_line(log_path, "not-a-dict") + dl.record_read("tfidf", "doc.md", 100, 200, path=log_path) + + result = dl.summarize_reads(log_path) + assert result == {"methods": {"tfidf": {"count": 1, "total_tokens": 200, "total_lines": 100}}, + "total_tokens": 200} + + +def test_summarize_reads_skips_non_numeric_tokens_or_lines(log_path: Path) -> None: + _append_raw_read_line(log_path, {"method": "tfidf", "tokens": "not-a-number", "lines": 100}) + dl.record_read("baseline", "doc.md", 100, 200, path=log_path) + + result = dl.summarize_reads(log_path) + assert result == {"methods": {"baseline": {"count": 1, "total_tokens": 200, "total_lines": 100}}, + "total_tokens": 200} + + # --------------------------------------------------------------------------- # path resolution diff --git a/tests/test_doc_index.py b/tests/test_doc_index.py index 5e9d7943..e77e3c4e 100644 --- a/tests/test_doc_index.py +++ b/tests/test_doc_index.py @@ -105,7 +105,37 @@ def test_retrieval_sections_grouped_at_inferred_level(self, tmp_path: Path): index = build_doc_index(f) assert index["section_level"] == 2 # "### A.1" (H3, off-level) stays inside "## Section A", not its own section. - assert [s["heading"] for s in index["retrieval_sections"]] == ["Section A", "Section B"] + # A leading `None`-heading entry captures "# Title" (real content before + # the first H2), which used to be silently dropped from retrieval_sections. + assert [s["heading"] for s in index["retrieval_sections"]] == [None, "Section A", "Section B"] + + def test_preamble_before_first_section_is_captured_not_dropped(self, tmp_path: Path): + """CodeRabbit PR #110: content before the first section-level + heading (a title, an intro paragraph) used to be invisible to + every retrieval_sections entry -- present in the finer-grained + sections list, but nowhere in the coarser one a retriever + actually reads.""" + f = _write(tmp_path) + index = build_doc_index(f) + preamble = index["retrieval_sections"][0] + assert preamble["heading"] is None + assert preamble["line_start"] == 1 + assert preamble["line_end"] == 2 # "# Title" + the blank line after it + assert preamble["empty"] is False + + def test_blank_only_preamble_is_not_captured(self, tmp_path: Path): + content = "\n\n## Section A\n\nBody of A.\n" + f = _write(tmp_path, content) + index = build_doc_index(f) + assert [s["heading"] for s in index["retrieval_sections"]] == ["Section A"] + + def test_adjacent_same_level_headings_produce_an_empty_section(self, tmp_path: Path): + content = "## Section A\n## Section B\n\nBody of B.\n" + f = _write(tmp_path, content) + index = build_doc_index(f) + by_heading = {s["heading"]: s for s in index["retrieval_sections"]} + assert by_heading["Section A"]["empty"] is True + assert by_heading["Section B"]["empty"] is False def test_headingless_document_has_no_retrieval_sections(self, tmp_path: Path): f = _write(tmp_path, "Just a paragraph, no headings at all.\n") @@ -174,6 +204,42 @@ def test_matches_real_parser_output(self, tmp_path: Path): assert infer_section_level(headings) == 5 +class TestLargeDocumentIntegration: + """CodeRabbit PR #110: infer_section_level's own docstring cites a real + failure at real scale (8 chapters on H5, one stray H3, ~6,601 lines) -- + but every existing test exercised the formula against a handful of + synthetic heading tuples, never the full build_doc_index pipeline at + anything close to that shape. Reproduces it end to end: real file I/O, + real heading parsing, real section splitting and hashing.""" + + def test_full_pipeline_at_real_bug_scale(self, tmp_path: Path): + chapter_body = "\n".join(f"Paragraph {i} of chapter filler text." for i in range(800)) + chapters = [f"##### Chapter {i}\n\n{chapter_body}\n" for i in range(1, 9)] + # A stray, numerically-shallower H3 dropped into the middle, exactly + # like the real PDF-conversion artifact this heuristic exists for. + content = "\n".join(chapters[:4]) + "\n### Stray Subsection\n\nbody\n" + "\n".join(chapters[4:]) + f = tmp_path / "large.md" + f.write_text(content, encoding="utf-8") + + assert len(content.split("\n")) > 6000 + + index = build_doc_index(f) + assert index["section_level"] == 5 + headings = [s["heading"] for s in index["retrieval_sections"]] + assert headings == [f"Chapter {i}" for i in range(1, 9)] + + # Correct line-range partitioning: every chapter's content stays + # inside its own section, none bleed into a "fake mega-section." + for section in index["retrieval_sections"]: + assert section["line_end"] > section["line_start"] + line_starts = [s["line_start"] for s in index["retrieval_sections"]] + assert line_starts == sorted(line_starts) + # Every section's hash is genuinely distinct real content, not the + # same value repeated (which would indicate a broken line-range + # computation collapsing sections together). + assert len({s["hash"] for s in index["retrieval_sections"]}) == 8 + + class TestDiffStaleSections: def test_returns_none_when_no_cache_exists(self, tmp_path: Path, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) @@ -196,6 +262,7 @@ def test_no_edit_reports_everything_unchanged(self, tmp_path: Path, monkeypatch) assert diff["structural_change"] is False assert diff["changed"] == [] assert {(e["heading"], e["line_start"]) for e in diff["unchanged"]} == { + (None, 1), ("Section A", 3), ("Section B", 11), } @@ -208,7 +275,40 @@ def test_editing_one_section_reports_only_that_one_changed(self, tmp_path: Path, diff = diff_stale_sections(f) assert diff["structural_change"] is False assert diff["changed"] == [{"heading": "Section B", "line_start": 11}] - assert diff["unchanged"] == [{"heading": "Section A", "line_start": 3}] + assert {(e["heading"], e["line_start"]) for e in diff["unchanged"]} == {(None, 1), ("Section A", 3)} + + def test_reordered_sections_with_no_text_edits_report_everything_unchanged( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110: sections used to be matched by position, so + a pure reorder with zero text changes reported every section as + changed. Hash-based matching (a multiset match, so duplicate + content is handled correctly) now recognizes moved-but-unedited + content as unchanged.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "## Section A\n\nBody of A.\n\n## Section B\n\nBody of B.\n" + reordered = "## Section B\n\nBody of B.\n\n## Section A\n\nBody of A.\n" + f = _write(tmp_path, content) + save_doc_index(f, build_doc_index(f)) + f.write_text(reordered, encoding="utf-8") + diff = diff_stale_sections(f) + assert diff["structural_change"] is False + assert diff["changed"] == [] + assert {e["heading"] for e in diff["unchanged"]} == {"Section A", "Section B"} + + def test_reorder_combined_with_a_real_edit_flags_only_the_edited_content( + self, tmp_path: Path, monkeypatch + ): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "## Section A\n\nBody of A.\n\n## Section B\n\nBody of B.\n" + reordered_and_edited = "## Section B\n\nBody of B.\n\n## Section A\n\nBody of A, edited.\n" + f = _write(tmp_path, content) + save_doc_index(f, build_doc_index(f)) + f.write_text(reordered_and_edited, encoding="utf-8") + diff = diff_stale_sections(f) + assert diff["structural_change"] is False + assert [e["heading"] for e in diff["changed"]] == ["Section A"] + assert [e["heading"] for e in diff["unchanged"]] == ["Section B"] def test_duplicate_headings_are_disambiguated_by_line_start(self, tmp_path: Path, monkeypatch): """CodeRabbit PR #109: heading text alone can't tell two identically @@ -239,7 +339,7 @@ def test_adding_a_retrieval_level_heading_is_a_structural_change(self, tmp_path: diff = diff_stale_sections(f) assert diff["structural_change"] is True assert diff["unchanged"] == [] - assert {e["heading"] for e in diff["changed"]} == {"Section A", "Section B", "Section C"} + assert {e["heading"] for e in diff["changed"]} == {None, "Section A", "Section B", "Section C"} class TestCachePersistence: @@ -271,6 +371,46 @@ def test_legacy_cache_with_matching_etag_but_old_schema_is_rebuilt_not_returned( assert "section_level" in index assert "retrieval_sections" in index + def test_intermediate_cache_missing_per_section_hash_is_rebuilt_not_returned( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110: retrieval_sections existed before per-section + hash did. A cache from that intermediate schema has both required + top-level fields, so the field-presence check alone lets it through + -- but every consumer that reads entry["hash"] (diff_stale_sections, + get_okf_status, the doc-index command's human formatter) then hits a + KeyError. Treated the same as any other schema mismatch: rebuilt.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + intermediate = build_doc_index(f) + for section in intermediate["retrieval_sections"]: + del section["hash"] + save_doc_index(f, intermediate) + + assert load_doc_index(f) is None + assert diff_stale_sections(f) is None + + index = get_or_build_doc_index(f) + assert index["cache_hit"] is False + assert all("hash" in s for s in index["retrieval_sections"]) + + def test_cache_missing_sections_field_is_rebuilt_not_returned(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: a matching-etag cache that has section_level + and retrieval_sections (both valid) but omits sections passed the + old field-presence check -- cmd_doc_index() then hits a KeyError at + index["sections"]. sections is now itself a required field.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + incomplete = build_doc_index(f) + del incomplete["sections"] + save_doc_index(f, incomplete) + + assert load_doc_index(f) is None + + index = get_or_build_doc_index(f) + assert index["cache_hit"] is False + assert "sections" in index + def test_cmd_doc_index_does_not_crash_on_a_legacy_cache(self, tmp_path: Path, capsys, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) @@ -295,6 +435,19 @@ def test_save_then_load_round_trips(self, tmp_path: Path, monkeypatch): assert loaded["etag"] == built["etag"] assert loaded["sections"] == built["sections"] + def test_save_returns_true_on_a_real_write(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: save_doc_index used to return None + unconditionally, giving a caller no way to distinguish an actual + write from a silent no-op outside a Studio project.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert save_doc_index(f, build_doc_index(f)) is True + + def test_save_returns_false_outside_a_studio_project(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + assert save_doc_index(f, build_doc_index(f)) is False + def test_load_returns_none_when_cache_is_stale(self, tmp_path: Path, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) @@ -469,8 +622,9 @@ def test_second_call_is_cache_hit(self, tmp_path: Path, monkeypatch): def test_cache_hit_preserves_previously_annotated_summary(self, tmp_path: Path, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) - get_or_build_doc_index(f) - assert annotate_section_summary(f, line_start=3, summary="Covers A.") is True + built = get_or_build_doc_index(f) + section_a_hash = next(s["hash"] for s in built["sections"] if s["line_start"] == 3) + assert annotate_section_summary(f, line_start=3, expected_hash=section_a_hash, summary="Covers A.") is True index = get_or_build_doc_index(f) assert index["cache_hit"] is True section_a = next(s for s in index["sections"] if s["heading"] == "Section A") @@ -479,8 +633,9 @@ def test_cache_hit_preserves_previously_annotated_summary(self, tmp_path: Path, def test_content_change_invalidates_and_drops_stale_summaries(self, tmp_path: Path, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) - get_or_build_doc_index(f) - annotate_section_summary(f, line_start=3, summary="Covers A.") + built = get_or_build_doc_index(f) + section_a_hash = next(s["hash"] for s in built["sections"] if s["line_start"] == 3) + annotate_section_summary(f, line_start=3, expected_hash=section_a_hash, summary="Covers A.") f.write_text(_SAMPLE + "\n## Section C\n") index = get_or_build_doc_index(f) assert index["cache_hit"] is False @@ -498,19 +653,57 @@ class TestAnnotateSectionSummary: def test_returns_false_when_no_cache_exists_yet(self, tmp_path: Path, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) - assert annotate_section_summary(f, line_start=3, summary="x") is False + assert annotate_section_summary(f, line_start=3, expected_hash="anything", summary="x") is False + + def test_returns_false_outside_a_studio_project(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + assert annotate_section_summary(f, line_start=3, expected_hash="anything", summary="x") is False def test_returns_false_for_unmatched_line_start(self, tmp_path: Path, monkeypatch): monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) get_or_build_doc_index(f) - assert annotate_section_summary(f, line_start=999, summary="x") is False + assert annotate_section_summary(f, line_start=999, expected_hash="anything", summary="x") is False + + def test_propagates_a_persistence_failure_instead_of_reporting_true(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: annotate_section_summary used to return True + unconditionally after calling save_doc_index, discarding whatever + save_doc_index actually reported -- an external caller (e.g. an + LLM summarization pass) would believe a summary was persisted when + the underlying write silently failed/no-opped.""" + import studio.utils.doc_index as di - def test_returns_true_and_persists_on_match(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + line_start = index["sections"][0]["line_start"] + expected_hash = index["sections"][0]["hash"] + + monkeypatch.setattr(di, "save_doc_index", lambda *_a, **_k: False) + result = annotate_section_summary(f, line_start=line_start, expected_hash=expected_hash, summary="x") + assert result is False + + def test_returns_false_on_hash_mismatch(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #110: a caller's expected_hash must match the + section's current hash, or the write is rejected -- otherwise a + document edited between read and write-back could silently + attach one section's summary to a different section's content + that now happens to occupy the same line_start.""" monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) get_or_build_doc_index(f) - assert annotate_section_summary(f, line_start=1, summary="The title.") is True + assert annotate_section_summary(f, line_start=3, expected_hash="stale-hash", summary="x") is False + cached = load_doc_index(f) + section_a = next(s for s in cached["sections"] if s["line_start"] == 3) + assert section_a["summary"] is None + + def test_returns_true_and_persists_on_match(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + built = get_or_build_doc_index(f) + title_hash = built["sections"][0]["hash"] + assert annotate_section_summary(f, line_start=1, expected_hash=title_hash, summary="The title.") is True cached = load_doc_index(f) assert cached["sections"][0]["summary"] == "The title." @@ -521,8 +714,9 @@ def test_updates_matching_retrieval_section_too(self, tmp_path: Path, monkeypatc relevant list for a future OKF-style summarizer) couldn't see it.""" monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) - get_or_build_doc_index(f) - assert annotate_section_summary(f, line_start=3, summary="Covers A.") is True + built = get_or_build_doc_index(f) + section_a_hash = next(s["hash"] for s in built["sections"] if s["line_start"] == 3) + assert annotate_section_summary(f, line_start=3, expected_hash=section_a_hash, summary="Covers A.") is True index = load_doc_index(f) retrieval_a = next(s for s in index["retrieval_sections"] if s["heading"] == "Section A") assert retrieval_a["summary"] == "Covers A." @@ -534,8 +728,9 @@ def test_off_level_heading_leaves_retrieval_sections_untouched(self, tmp_path: P retrieval section to touch.""" monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) - get_or_build_doc_index(f) - assert annotate_section_summary(f, line_start=7, summary="About A.1.") is True + built = get_or_build_doc_index(f) + a1_hash = next(s["hash"] for s in built["sections"] if s["line_start"] == 7) + assert annotate_section_summary(f, line_start=7, expected_hash=a1_hash, summary="About A.1.") is True index = load_doc_index(f) a1 = next(s for s in index["sections"] if s["heading"] == "A.1") assert a1["summary"] == "About A.1." @@ -559,21 +754,24 @@ def test_concurrent_annotations_of_different_sections_do_not_lose_either_update( monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) index = get_or_build_doc_index(f) - line_a = index["sections"][1]["line_start"] # "Section A" - line_b = index["sections"][3]["line_start"] # "Section B" + section_a = index["sections"][1] # "Section A" + section_b = index["sections"][3] # "Section B" + line_a, hash_a = section_a["line_start"], section_a["hash"] + line_b, hash_b = section_b["line_start"], section_b["hash"] original_save = di.save_doc_index def slow_save(path, saved_index): time_module.sleep(0.1) - original_save(path, saved_index) + return original_save(path, saved_index) monkeypatch.setattr(di, "save_doc_index", slow_save) results: dict = {} + hashes = {line_a: hash_a, line_b: hash_b} def run(line_start: int, summary: str) -> None: - results[line_start] = di.annotate_section_summary(f, line_start, summary) + results[line_start] = di.annotate_section_summary(f, line_start, hashes[line_start], summary) t1 = threading.Thread(target=run, args=(line_a, "Summary A")) t2 = threading.Thread(target=run, args=(line_b, "Summary B")) @@ -636,6 +834,16 @@ def test_missing_file(self, tmp_path: Path, capsys): out = json.loads(capsys.readouterr().out) assert out["status"] == "ERROR" + def test_missing_required_argument_emits_json_error_not_a_plain_text_banner(self, capsys): + """CodeRabbit PR #111: cmd_doc_index uses JsonSafeArgumentParser, so + omitting the required positional must still emit the project's own + --json ERROR contract (via parse_args_or_json_error), not argparse's + default usage banner + SystemExit.""" + rc = cmd_doc_index([]) # file omitted + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + def test_unreadable_file_reports_a_clean_error_not_a_raw_traceback( self, tmp_path: Path, capsys, monkeypatch ): @@ -674,8 +882,8 @@ def test_json_output_exposes_retrieval_sections(self, tmp_path: Path, capsys, mo assert rc == 0 out = json.loads(capsys.readouterr().out) assert out["section_level"] == 2 - assert out["retrieval_section_count"] == 2 - assert [s["heading"] for s in out["retrieval_sections"]] == ["Section A", "Section B"] + assert out["retrieval_section_count"] == 3 + assert [s["heading"] for s in out["retrieval_sections"]] == [None, "Section A", "Section B"] assert "hash" in out["retrieval_sections"][0] def test_non_utf8_file_reports_a_clean_error_not_a_raw_traceback(self, tmp_path: Path, capsys, monkeypatch): @@ -716,7 +924,8 @@ def test_human_output_lists_retrieval_sections(self, tmp_path: Path, capsys, mon set_json_mode(orig) assert rc == 0 out = capsys.readouterr().out - assert "Retrieval sections (level 2, 2 section(s))" in out + assert "Retrieval sections (level 2, 3 section(s))" in out + assert "(preamble)" in out assert "Section A" in out assert "Section B" in out @@ -778,9 +987,11 @@ def test_human_output_mode_with_section_summary(self, tmp_path: Path, capsys, mo monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) f = _write(tmp_path) + built = get_or_build_doc_index(f) + section_a_hash = next(s["hash"] for s in built["sections"] if s["line_start"] == 3) cmd_doc_index([str(f)]) capsys.readouterr() - assert annotate_section_summary(f, line_start=3, summary="Covers A.") is True + assert annotate_section_summary(f, line_start=3, expected_hash=section_a_hash, summary="Covers A.") is True orig = is_json_mode() set_json_mode(False) try: diff --git a/tests/test_heading_nav.py b/tests/test_heading_nav.py new file mode 100644 index 00000000..76ad02d0 --- /dev/null +++ b/tests/test_heading_nav.py @@ -0,0 +1,159 @@ +"""Tests for heading-nav retrieval over a document's retrieval sections +(heading_nav.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from studio.commands.heading_nav import cmd_heading_nav +from studio.utils.heading_nav import find_sections + +_SAMPLE = ( + "## Introduction\n\n" + "This section introduces the KAPING framework for knowledge graphs.\n\n" + "## Related Work\n\n" + "This section covers unrelated background material with no overlap.\n\n" + "## Conclusion\n\n" + "A short closing section.\n" +) + + +def _write(tmp_path: Path, content: str = _SAMPLE, name: str = "doc.md") -> Path: + f = tmp_path / name + f.write_text(content, encoding="utf-8") + return f + + +class TestFindSections: + def test_headingless_document_returns_no_matches(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "Just a paragraph, no headings.\n") + result = find_sections(f, "anything") + assert result == {"matches": [], "first_match": None} + + def test_exact_term_present_in_one_section(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = find_sections(f, "KAPING") + assert [m["heading"] for m in result["matches"]] == ["Introduction"] + assert result["matches"][0]["hit_count"] == 1 + assert result["first_match"] == result["matches"][0] + + def test_wording_mismatch_is_a_hard_failure_zero_hits(self, tmp_path: Path, monkeypatch): + """Mirrors findings.md's real "making up" case: this method has no + semantic fallback, so a query phrased differently than the source's + own vocabulary returns nothing, even though a related word exists.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## Section\n\nThe model sometimes hallucinates facts.\n") + result = find_sections(f, "making up") + assert result == {"matches": [], "first_match": None} + + def test_is_case_insensitive(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = find_sections(f, "kaping") + assert result["first_match"]["heading"] == "Introduction" + + def test_multiple_occurrences_in_one_section_are_counted(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## A\n\nwidget widget widget\n") + result = find_sections(f, "widget") + assert result["matches"][0]["hit_count"] == 3 + + def test_term_present_in_multiple_sections_lists_all_in_document_order(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## A\n\nshared here\n\n## B\n\nshared there too\n") + result = find_sections(f, "shared") + assert [m["heading"] for m in result["matches"]] == ["A", "B"] + assert result["first_match"]["heading"] == "A" + + def test_empty_query_returns_no_matches(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert find_sections(f, " ") == {"matches": [], "first_match": None} + + def test_hit_inside_a_fenced_code_block_does_not_count(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: a term appearing only inside a fenced code + sample (a command, a variable name) must not count as a prose hit + -- otherwise an incidental code-sample match could make this + method pick a section on nothing but a coincidental identifier.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "## A\n\nSome prose here.\n\n```bash\nwidget --flag\n```\n" + f = _write(tmp_path, content) + result = find_sections(f, "widget") + assert result == {"matches": [], "first_match": None} + + def test_hit_outside_a_fenced_code_block_still_counts(self, tmp_path: Path, monkeypatch): + """The other side of the fence fix: a real prose hit alongside an + unrelated code block must still be found.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "## A\n\nThe widget is mentioned here in prose.\n\n```bash\necho hello\n```\n" + f = _write(tmp_path, content) + result = find_sections(f, "widget") + assert result["matches"][0]["hit_count"] == 1 + + +class TestCmdHeadingNav: + def test_missing_file(self, tmp_path: Path, capsys): + rc = cmd_heading_nav([str(tmp_path / "nope.md"), "query"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_missing_required_argument_emits_json_error_not_a_plain_text_banner(self, capsys): + """CodeRabbit PR #111: cmd_heading_nav now uses JsonSafeArgumentParser + (like every other single-file command), so omitting a required + positional must still emit the project's own --json ERROR contract, + not argparse's default usage banner + SystemExit.""" + rc = cmd_heading_nav([]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_non_utf8_file_reports_a_clean_error_not_a_raw_traceback(self, tmp_path: Path, capsys): + f = tmp_path / "bad.md" + f.write_bytes(b"# Title\n\xff\xfe not valid utf-8\n") + rc = cmd_heading_nav([str(f), "query"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_basic_json_output(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + rc = cmd_heading_nav([str(f), "KAPING"]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["first_match"]["heading"] == "Introduction" + + def test_human_output_with_matches(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_heading_nav([str(f), "KAPING"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "Introduction" in capsys.readouterr().out + + def test_human_output_no_matches(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_heading_nav([str(f), "zzzznomatch"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "no semantic fallback" in capsys.readouterr().out diff --git a/tests/test_okf.py b/tests/test_okf.py new file mode 100644 index 00000000..95bb6b9d --- /dev/null +++ b/tests/test_okf.py @@ -0,0 +1,671 @@ +"""Tests for the local, regenerable OKF bundle (okf.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from studio.commands.okf import cmd_okf_status +from studio.utils.doc_index import get_or_build_doc_index +from studio.utils.okf import ( + _okf_bundle_dir, + get_okf_status, + load_okf_manifest, + save_okf_manifest, + write_concept_file, +) + +_SAMPLE = ( + "## Introduction\n\n" + "Body of the introduction.\n\n" + "## Details\n\n" + "Body of details.\n\n" + "## Details\n\n" + "Body of the second details section (duplicate heading).\n" +) + + +def _write(tmp_path: Path, content: str = _SAMPLE, name: str = "doc.md") -> Path: + f = tmp_path / name + f.write_text(content, encoding="utf-8") + return f + + +class TestGetOkfStatus: + def test_unavailable_outside_a_studio_project(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + status = get_okf_status(f) + assert status == {"available": False, "bundle_dir": None, "entries": []} + + def test_all_sections_missing_before_anything_is_written(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + status = get_okf_status(f) + assert status["available"] is True + assert len(status["entries"]) == 3 + assert all(e["status"] == "missing" for e in status["entries"]) + + def test_duplicate_headings_get_distinct_concept_filenames(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + status = get_okf_status(f) + filenames = [e["concept_file"] for e in status["entries"]] + assert len(filenames) == len(set(filenames)) + assert filenames == ["01-introduction.md", "02-details.md", "03-details.md"] + + def test_written_section_reports_current(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + assert write_concept_file( + f, intro["line_start"], description="Covers the intro.", body="Summary here.", generated_by="test" + ) is True + + status = get_okf_status(f) + by_heading = {e["heading"]: e for e in status["entries"]} + assert by_heading["Introduction"]["status"] == "current" + assert by_heading["Details"]["status"] == "missing" # untouched, both of them + + def test_deleting_a_written_concept_file_reports_missing_not_current(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #110: a manifest entry's hash still matches after + its concept file is deleted out from under it (a manual cleanup, + say) -- the hash alone can't prove the file it points at survives, + so status must fall back to missing rather than current.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="d", body="b") + assert get_okf_status(f)["entries"][0]["status"] == "current" + + bundle_dir = _okf_bundle_dir(f) + (bundle_dir / "01-introduction.md").unlink() + + status = get_okf_status(f) + by_heading = {e["heading"]: e for e in status["entries"]} + assert by_heading["Introduction"]["status"] == "missing" + + def test_corrupted_concept_file_reports_missing_not_current(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: a manifest entry's hash still matches even + after its concept file is truncated/corrupted in place -- physical + presence alone can't prove the content is real, so status must + fall back to missing rather than trusting a file never read.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="d", body="b") + assert get_okf_status(f)["entries"][0]["status"] == "current" + + bundle_dir = _okf_bundle_dir(f) + (bundle_dir / "01-introduction.md").write_text("garbage, not frontmatter", encoding="utf-8") + + status = get_okf_status(f) + by_heading = {e["heading"]: e for e in status["entries"]} + assert by_heading["Introduction"]["status"] == "missing" + + def test_editing_the_source_after_writing_reports_stale(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="d", body="b") + + f.write_text(_SAMPLE.replace("Body of the introduction.", "Edited intro body."), encoding="utf-8") + status = get_okf_status(f) + by_heading = {e["heading"]: e for e in status["entries"]} + assert by_heading["Introduction"]["status"] == "stale" + + def test_stale_entry_keeps_its_own_written_concept_file_not_a_new_name( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110 (round 3): a heading rename changes the + section's hash (stale), but the *old* concept file, written under + the *old* heading's filename, still exists on disk. Reporting a + freshly-derived filename from the new heading would point + okf-status/index.md at a file that was never written.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="d", body="b") + original_concept_file = get_okf_status(f)["entries"][0]["concept_file"] + assert original_concept_file == "01-introduction.md" + + renamed = _SAMPLE.replace("## Introduction", "## Intro") + f.write_text(renamed, encoding="utf-8") + + status = get_okf_status(f) + renamed_entry = status["entries"][0] + assert renamed_entry["heading"] == "Intro" + assert renamed_entry["status"] == "stale" + assert renamed_entry["concept_file"] == original_concept_file + bundle_dir = _okf_bundle_dir(f) + assert (bundle_dir / renamed_entry["concept_file"]).is_file() + + def test_stale_entry_with_a_deleted_concept_file_reports_missing( + self, tmp_path: Path, monkeypatch + ): + """The other side of the fix above: if the stale entry's own + concept file is gone (or corrupted), it must report missing, not + stale -- a caller can't be pointed at a file that isn't there.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="d", body="b") + bundle_dir = _okf_bundle_dir(f) + (bundle_dir / "01-introduction.md").unlink() + + renamed = _SAMPLE.replace("## Introduction", "## Intro") + f.write_text(renamed, encoding="utf-8") + + status = get_okf_status(f) + assert status["entries"][0]["status"] == "missing" + + def test_new_section_landing_on_a_moved_sections_old_line_start_is_not_stale( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110 (round 4): a moved section's manifest entry is + only removed from the hash pool, not from by_line_start -- a + completely different, brand-new section that lands exactly on that + vacated line_start could inherit the moved section's concept_file + and report "stale" instead of "missing", pointing index.md at a + summary that was never written for it.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = ( + "## Alpha\n\nAlpha body line one.\nAlpha body line two.\n\n" + "## Beta\n\nBeta body.\n" + ) + f = _write(tmp_path, content) + index = get_or_build_doc_index(f) + beta = index["retrieval_sections"][1] + assert beta["heading"] == "Beta" + write_concept_file(f, beta["line_start"], description="d", body="b") + beta_concept_file = get_okf_status(f)["entries"][1]["concept_file"] + + # Insert a new section ("Gamma") the same size as what it displaces, + # so it lands precisely on Beta's *old* line_start while Beta itself + # (unchanged content) shifts further down. + moved = content.replace( + "## Beta", "## Gamma\n\nGamma body.\n\n## Beta" + ) + f.write_text(moved, encoding="utf-8") + new_index = get_or_build_doc_index(f) + gamma = next(s for s in new_index["retrieval_sections"] if s["heading"] == "Gamma") + assert gamma["line_start"] == beta["line_start"] # landed exactly on Beta's old spot + + status = get_okf_status(f) + by_heading = {e["heading"]: e for e in status["entries"]} + assert by_heading["Gamma"]["status"] == "missing" + assert by_heading["Gamma"]["concept_file"] != beta_concept_file + assert by_heading["Beta"]["status"] == "current" + assert by_heading["Beta"]["concept_file"] == beta_concept_file + + def test_a_section_that_moved_without_changing_reports_current_not_missing( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110 (round 2): inserting a new section between two + already-written ones shifts everything after it to a new + line_start -- content-identical sections must reconcile to their + existing manifest entry by hash and keep their original + concept_file, not report "missing" and force a needless + re-summarization. Section lengths differ deliberately so no old + line_start numerically collides with an unrelated new one.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = ( + "## Alpha\n\nAlpha body line one.\nAlpha body line two.\n\n" + "## Beta\n\nBeta body.\n" + ) + f = _write(tmp_path, content) + index = get_or_build_doc_index(f) + beta = index["retrieval_sections"][1] + assert beta["heading"] == "Beta" + write_concept_file(f, beta["line_start"], description="d", body="b") + beta_concept_file = get_okf_status(f)["entries"][1]["concept_file"] + + # Insert a differently-sized section between Alpha and Beta -- Beta's + # own text is untouched, but its line_start shifts. + moved = content.replace( + "## Beta", "## Gamma\n\nGamma body.\n\n## Beta" + ) + f.write_text(moved, encoding="utf-8") + new_index = get_or_build_doc_index(f) + new_beta = next(s for s in new_index["retrieval_sections"] if s["heading"] == "Beta") + assert new_beta["line_start"] != beta["line_start"] + + status = get_okf_status(f) + by_heading = {e["heading"]: e for e in status["entries"]} + assert by_heading["Beta"]["status"] == "current" + assert by_heading["Beta"]["concept_file"] == beta_concept_file + assert by_heading["Alpha"]["status"] == "missing" + assert by_heading["Gamma"]["status"] in ("missing", "stale") + + def test_duplicate_content_sections_each_reconcile_to_a_distinct_entry( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110 (round 2): two sections with byte-identical + text share one content hash -- reconciling both to the *same* + manifest entry after a reorder would silently drop one's own + summary. Each must claim its own entry from the hash pool.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + dup_content = ( + "## Details\n\nSame body text.\n\n" + "## Details\n\nSame body text.\n" + ) + f = _write(tmp_path, dup_content) + index = get_or_build_doc_index(f) + details_sections = index["retrieval_sections"] + assert len(details_sections) == 2 + assert details_sections[0]["hash"] == details_sections[1]["hash"] + + write_concept_file(f, details_sections[0]["line_start"], description="first", body="b1") + write_concept_file(f, details_sections[1]["line_start"], description="second", body="b2") + before = get_okf_status(f)["entries"] + before_files = {e["line_start"]: e["concept_file"] for e in before} + + # Reorder by prepending a new section -- both Details sections shift + # by the same offset, hashes unchanged. + f.write_text("## Preamble\n\nPreamble body line only.\n\n" + dup_content, encoding="utf-8") + after = get_okf_status(f)["entries"] + after_details = [e for e in after if e["heading"] == "Details"] + assert len(after_details) == 2 + assert all(e["status"] == "current" for e in after_details) + after_files = sorted(e["concept_file"] for e in after_details) + assert after_files == sorted(before_files.values()) + + +class TestWriteConceptFile: + def test_returns_false_outside_a_studio_project(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + assert write_concept_file(f, 1, description="d", body="b") is False + + def test_returns_false_for_unmatched_line_start(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert write_concept_file(f, 9999, description="d", body="b") is False + + def test_propagates_a_persistence_failure_instead_of_reporting_true(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: write_concept_file used to return True + unconditionally after calling save_okf_manifest, discarding + whatever save_okf_manifest actually reported.""" + import studio.utils.okf as okf_module + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + line_start = index["retrieval_sections"][0]["line_start"] + + monkeypatch.setattr(okf_module, "save_okf_manifest", lambda *_a, **_k: False) + assert write_concept_file(f, line_start, description="d", body="b") is False + + def test_writes_concept_file_with_frontmatter_and_body(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + assert write_concept_file( + f, intro["line_start"], description="Covers the intro.", body="Real summary body.", generated_by="claude" + ) is True + + status = get_okf_status(f) + bundle_dir = Path(status["bundle_dir"]) + concept_path = bundle_dir / "01-introduction.md" + assert concept_path.is_file() + content = concept_path.read_text(encoding="utf-8") + assert 'title: "Introduction"' in content + assert 'description: "Covers the intro."' in content + assert 'by: "claude"' in content + assert "Real summary body." in content + + def test_writes_a_concept_file_for_the_preamble_section(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #110: the synthetic preamble section (heading=None, + content before a document's first real heading) must slugify to a + real, readable filename/title instead of crashing on a heading + that was never a real string.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "# Title\n\nAn intro paragraph.\n\n" + _SAMPLE + f = _write(tmp_path, content) + index = get_or_build_doc_index(f) + preamble = index["retrieval_sections"][0] + assert preamble["heading"] is None + assert write_concept_file( + f, preamble["line_start"], description="The preamble.", body="Body.", generated_by="claude" + ) is True + + status = get_okf_status(f) + bundle_dir = Path(status["bundle_dir"]) + concept_path = bundle_dir / "01-preamble.md" + assert concept_path.is_file() + assert 'title: "(preamble)"' in concept_path.read_text(encoding="utf-8") + + def test_writes_and_updates_index_md(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="Covers the intro.", body="Summary.") + + status = get_okf_status(f) + index_md = (Path(status["bundle_dir"]) / "index.md").read_text(encoding="utf-8") + assert "[Introduction](01-introduction.md) - Covers the intro." in index_md + + def test_index_md_keeps_a_moved_sections_description_after_reorder( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110 (round 3): index.md's description lookup used + to be keyed by the *current* line_start, but a section resolved as + "current" after a reorder keeps its *original* manifest entry + (see get_okf_status) -- whose line_start is the *old* one. Looking + it up by the new line_start silently misses it and index.md shows + "(no summary yet)" for a section that really has a description.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + details = index["retrieval_sections"][1] + assert details["heading"] == "Details" + write_concept_file(f, details["line_start"], description="Covers details.", body="b") + + # Grow the Introduction section (position 1) -- Details' line_start + # shifts, its content (and hash) don't, so it resolves as current + # via the reorder-tolerant hash match, keeping its own entry. + grown = _SAMPLE.replace( + "Body of the introduction.\n\n", "Body of the introduction.\n\nMore intro text.\n\n" + ) + f.write_text(grown, encoding="utf-8") + + # Trigger an index.md regeneration via an unrelated write. + new_index = get_or_build_doc_index(f) + intro = new_index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="Covers the intro.", body="a") + + status = get_okf_status(f) + index_md = (Path(status["bundle_dir"]) / "index.md").read_text(encoding="utf-8") + assert "Covers details." in index_md + assert "(no summary yet)" not in index_md + + def test_new_section_does_not_steal_a_moved_sections_concept_filename( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #110 (round 4): concept_filename was derived purely + from the section's *current* position/heading, computed outside the + lock. If a reorder leaves a brand-new section at the same + position+heading a different, already-written (moved) section now + occupies, the naive filename collides and atomic_write_text + silently replaces the moved section's real summary.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = ( + "## Alpha\n\nAlpha body one.\nAlpha body two.\n\n" + "## Details\n\nDetails body A.\n" + ) + f = _write(tmp_path, content) + index = get_or_build_doc_index(f) + details = index["retrieval_sections"][1] + assert details["heading"] == "Details" + write_concept_file(f, details["line_start"], description="Original.", body="a") + original_concept_file = get_okf_status(f)["entries"][1]["concept_file"] + assert original_concept_file == "02-details.md" + + # Insert a brand-new "Details" section ahead of the original -- + # the new one now sits at position 2 (the exact position/heading + # combination that used to name the original's file), while the + # original (unchanged content) shifts to position 3 and resolves + # via hash match, keeping its own file. + reordered = content.replace( + "## Details", "## Details\n\nDetails body NEW.\n\n## Details", 1 + ) + f.write_text(reordered, encoding="utf-8") + new_index = get_or_build_doc_index(f) + new_details = new_index["retrieval_sections"][1] + moved_original = new_index["retrieval_sections"][2] + assert new_details["line_start"] == details["line_start"] # took over the old slot + assert moved_original["line_start"] != details["line_start"] # original shifted + + write_concept_file(f, new_details["line_start"], description="New one.", body="b") + + status = get_okf_status(f) + by_line_start = {e["line_start"]: e for e in status["entries"]} + original_after = next(e for e in status["entries"] if e["status"] == "current" + and e["concept_file"] == original_concept_file) + assert original_after["status"] == "current" + new_entry = by_line_start[new_details["line_start"]] + assert new_entry["concept_file"] != original_concept_file + + bundle_dir = _okf_bundle_dir(f) + original_content = (bundle_dir / original_concept_file).read_text(encoding="utf-8") + assert "Original." in original_content # not clobbered by the new write + + def test_second_write_does_not_duplicate_manifest_entries(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="First.", body="a") + write_concept_file(f, intro["line_start"], description="Second.", body="b") + + manifest = load_okf_manifest(f) + matching = [e for e in manifest["entries"] if e["line_start"] == intro["line_start"]] + assert len(matching) == 1 + assert matching[0]["description"] == "Second." + + def test_rewriting_after_a_source_edit_clears_stale_status(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + intro = index["retrieval_sections"][0] + write_concept_file(f, intro["line_start"], description="d", body="b") + f.write_text(_SAMPLE.replace("Body of the introduction.", "Edited."), encoding="utf-8") + assert get_okf_status(f)["entries"][0]["status"] == "stale" + + write_concept_file(f, intro["line_start"], description="d2", body="b2") + assert get_okf_status(f)["entries"][0]["status"] == "current" + + +class TestBundleDirLookup: + def test_lookup_error_means_no_crash_and_unavailable(self, tmp_path: Path, monkeypatch): + """An OSError from find_studio_directory (e.g. an unreadable parent + directory) must degrade to 'unavailable', not raise -- and it must + be logged, not silently swallowed.""" + def _raise(_start_path): + raise OSError("permission denied") + + monkeypatch.setattr("studio.utils.files.find_studio_directory", _raise) + f = _write(tmp_path) + assert get_okf_status(f) == {"available": False, "bundle_dir": None, "entries": []} + + def test_save_manifest_returns_false_outside_a_studio_project(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + assert save_okf_manifest(f, {"entries": []}) is False + + +class TestLoadOkfManifest: + def test_returns_none_when_no_manifest_exists(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert load_okf_manifest(f) is None + + def test_returns_none_outside_a_studio_project(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + assert load_okf_manifest(f) is None + + def test_returns_none_on_corrupt_manifest(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + write_concept_file(f, index["retrieval_sections"][0]["line_start"], description="d", body="b") + status = get_okf_status(f) + manifest_path = Path(status["bundle_dir"]) / "manifest.json" + manifest_path.write_text("{not valid json", encoding="utf-8") + assert load_okf_manifest(f) is None + + def test_returns_none_when_top_level_is_not_a_dict(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #110/#111 (both independently flagged this): a + manifest that decodes to valid JSON but isn't the expected object + shape (e.g. a bare list) must be treated the same as a + corrupt/absent one, not passed through for a reader to fail on.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + write_concept_file(f, index["retrieval_sections"][0]["line_start"], description="d", body="b") + status = get_okf_status(f) + manifest_path = Path(status["bundle_dir"]) / "manifest.json" + manifest_path.write_text("[]", encoding="utf-8") + assert load_okf_manifest(f) is None + + def test_returns_none_when_entries_is_not_a_list(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + write_concept_file(f, index["retrieval_sections"][0]["line_start"], description="d", body="b") + status = get_okf_status(f) + manifest_path = Path(status["bundle_dir"]) / "manifest.json" + manifest_path.write_text(json.dumps({"entries": "not-a-list"}), encoding="utf-8") + assert load_okf_manifest(f) is None + + def test_returns_none_when_an_entry_is_missing_a_required_field(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #110/#111 (both independently flagged this): an + entry missing a required field (hand-edited, or a future/older + schema) used to reach get_okf_status()'s dict comprehensions as an + unhandled KeyError.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + write_concept_file(f, index["retrieval_sections"][0]["line_start"], description="d", body="b") + status = get_okf_status(f) + manifest_path = Path(status["bundle_dir"]) / "manifest.json" + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + del manifest["entries"][0]["line_start"] + manifest_path.write_text(json.dumps(manifest), encoding="utf-8") + assert load_okf_manifest(f) is None + # get_okf_status must not crash either -- it falls back to "no manifest". + assert all(e["status"] == "missing" for e in get_okf_status(f)["entries"]) + + def test_empty_entries_list_is_a_valid_manifest(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert save_okf_manifest(f, {"entries": []}) is True + assert load_okf_manifest(f) == {"entries": []} + + def test_returns_none_when_a_field_has_the_wrong_type(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: {"line_start": [], ...} passes a presence-only + shape check but then raises TypeError when get_okf_status() uses + the unhashable list as a dict key -- field values must be + type-checked, not just present.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + write_concept_file(f, index["retrieval_sections"][0]["line_start"], description="d", body="b") + status = get_okf_status(f) + manifest_path = Path(status["bundle_dir"]) / "manifest.json" + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + manifest["entries"][0]["line_start"] = [] + manifest_path.write_text(json.dumps(manifest), encoding="utf-8") + assert load_okf_manifest(f) is None + # get_okf_status must not crash either -- it falls back to "no manifest". + assert all(e["status"] == "missing" for e in get_okf_status(f)["entries"]) + + +class TestGetOkfStatusReorderTolerance: + def test_content_inserted_above_an_unchanged_section_keeps_it_current(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: a section's own line_start shifts whenever + earlier content changes size, even without any structural change. + Since matching is primarily by content hash (see + get_okf_status's docstring), a section genuinely unchanged in + content must not report "missing" just because something above it + grew and shifted its line_start.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + details = index["retrieval_sections"][1] # "Details", position 2 + assert details["heading"] == "Details" + write_concept_file(f, details["line_start"], description="d", body="b") + assert get_okf_status(f)["entries"][1]["status"] == "current" + + # Grow the Introduction section (position 1) without touching Details -- + # Details' line_start shifts, but its content (and hash) don't. + grown = _SAMPLE.replace( + "Body of the introduction.\n\n", "Body of the introduction.\n\nMore intro text.\n\n" + ) + f.write_text(grown, encoding="utf-8") + + status = get_okf_status(f) + assert status["entries"][1]["heading"] == "Details" + assert status["entries"][1]["status"] == "current" + + +class TestCmdOkfStatus: + def test_missing_file(self, tmp_path: Path, capsys): + rc = cmd_okf_status([str(tmp_path / "nope.md")]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_directory_as_file_argument_is_rejected(self, tmp_path: Path, capsys): + rc = cmd_okf_status([str(tmp_path)]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_missing_required_argument_emits_json_error_not_a_plain_text_banner(self, capsys): + """CodeRabbit PR #110: an argparse parsing failure used to bypass + this project's own --json output contract entirely.""" + rc = cmd_okf_status([]) # file omitted + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_headingless_document_reports_zero_entries(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "Just a paragraph, no headings at all.\n") + rc = cmd_okf_status([str(f)]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["available"] is True + assert out["entries"] == [] + + def test_basic_json_output(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + rc = cmd_okf_status([str(f)]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["available"] is True + assert len(out["entries"]) == 3 + + def test_human_output_available(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_okf_status([str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "3 missing" in out + assert "Introduction" in out + + def test_human_output_unavailable(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_okf_status([str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "unavailable" in capsys.readouterr().out diff --git a/tests/test_read_gate.py b/tests/test_read_gate.py new file mode 100644 index 00000000..ed15c4a8 --- /dev/null +++ b/tests/test_read_gate.py @@ -0,0 +1,112 @@ +"""Tests for the large-read confirmation gate (read_gate.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from studio.commands.read_gate import cmd_read_gate +from studio.utils.read_gate import DEFAULT_GATE_THRESHOLD_LINES, check_gate + + +class TestCheckGate: + def test_below_threshold_needs_no_confirmation(self): + result = check_gate(100, threshold=5000) + assert result == {"needs_confirmation": False, "total_lines": 100, "threshold": 5000} + + def test_above_threshold_needs_confirmation(self): + result = check_gate(8925, threshold=5000) + assert result == {"needs_confirmation": True, "total_lines": 8925, "threshold": 5000} + + def test_exactly_at_threshold_needs_no_confirmation(self): + """Real, tested boundary: this is a "crosses" threshold, not "reaches" -- a + document exactly at the threshold hasn't gone over it yet.""" + result = check_gate(5000, threshold=5000) + assert result["needs_confirmation"] is False + + def test_uses_the_documented_default_threshold(self): + result = check_gate(8925) + assert result["threshold"] == DEFAULT_GATE_THRESHOLD_LINES == 5000 + + def test_negative_total_lines_is_clamped_to_zero(self): + result = check_gate(-5) + assert result == {"needs_confirmation": False, "total_lines": 0, "threshold": DEFAULT_GATE_THRESHOLD_LINES} + + +class TestCmdReadGate: + def test_missing_file(self, tmp_path: Path, capsys): + rc = cmd_read_gate([str(tmp_path / "nope.md")]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_missing_required_argument_emits_json_error_not_a_plain_text_banner(self, capsys): + """CodeRabbit PR #111: cmd_read_gate now uses JsonSafeArgumentParser + (like every other single-file command), so omitting a required + positional must still emit the project's own --json ERROR contract, + not argparse's default usage banner + SystemExit.""" + rc = cmd_read_gate([]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_non_utf8_file_reports_a_clean_error_not_a_raw_traceback(self, tmp_path: Path, capsys): + f = tmp_path / "bad.md" + f.write_bytes(b"# Title\n\xff\xfe not valid utf-8\n") + rc = cmd_read_gate([str(f)]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_basic_json_output(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = tmp_path / "doc.md" + f.write_text("## A\n\nshort content\n", encoding="utf-8") + rc = cmd_read_gate([str(f)]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["needs_confirmation"] is False + assert out["threshold"] == DEFAULT_GATE_THRESHOLD_LINES + + def test_custom_threshold_flag(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = tmp_path / "doc.md" + f.write_text("## A\n\n" + "\n".join(f"line {i}" for i in range(20)) + "\n", encoding="utf-8") + rc = cmd_read_gate([str(f), "--threshold", "10"]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["needs_confirmation"] is True + assert out["threshold"] == 10 + + def test_human_output_needs_confirmation(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = tmp_path / "doc.md" + f.write_text("## A\n\n" + "\n".join(f"line {i}" for i in range(20)) + "\n", encoding="utf-8") + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_read_gate([str(f), "--threshold", "10"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "needs confirmation" in capsys.readouterr().out + + def test_human_output_no_confirmation_needed(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = tmp_path / "doc.md" + f.write_text("## A\n\nshort content\n", encoding="utf-8") + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_read_gate([str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "no confirmation needed" in capsys.readouterr().out diff --git a/tests/test_tfidf.py b/tests/test_tfidf.py new file mode 100644 index 00000000..7baabd92 --- /dev/null +++ b/tests/test_tfidf.py @@ -0,0 +1,243 @@ +"""Tests for TF-IDF scoring over a document's retrieval sections (tfidf.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from studio.commands.tfidf import cmd_tfidf_score +from studio.utils.tfidf import score_sections, tokenize + +_SAMPLE = ( + "## Introduction\n\n" + "This section introduces the KAPING framework for knowledge graphs.\n\n" + "## Related Work\n\n" + "This section covers unrelated background material with no overlap.\n\n" + "## Conclusion\n\n" + "A short closing section.\n" +) + + +def _write(tmp_path: Path, content: str = _SAMPLE, name: str = "doc.md") -> Path: + f = tmp_path / name + f.write_text(content, encoding="utf-8") + return f + + +class TestTokenize: + def test_lowercases_and_splits_on_non_alphanumeric(self): + assert tokenize("KAPING Framework-2023!") == ["kaping", "framework", "2023"] + + def test_drops_tokens_shorter_than_three_chars(self): + assert tokenize("making up things") == ["making", "things"] + + def test_empty_text_returns_empty_list(self): + assert tokenize("") == [] + + +class TestScoreSections: + def test_headingless_document_returns_empty_result(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "Just a paragraph, no headings.\n") + result = score_sections(f, "anything") + assert result == {"ranked": [], "margin": None, "unambiguous": False} + + def test_exact_score_tie_has_margin_one_and_is_not_unambiguous(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #110: two sections with identical positive scores + fall through to margin == top/second == 1.0, unambiguous == False + -- correct by inspection, but previously unpinned by any test.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "## A\n\nshared\n\n## B\n\nshared\n" + f = _write(tmp_path, content) + result = score_sections(f, "shared") + assert result["margin"] == 1.0 + assert result["unambiguous"] is False + + def test_empty_query_scores_everything_zero(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = score_sections(f, "") + assert result["margin"] is None + assert result["unambiguous"] is False + assert all(r["score"] == 0 for r in result["ranked"]) + + def test_distinctive_rare_term_is_unambiguous(self, tmp_path: Path, monkeypatch): + """Mirrors the real KAPING case from findings.md: a rare term that + appears in exactly one section scores that section positively and + every other section exactly zero.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = score_sections(f, "KAPING") + assert result["unambiguous"] is True + assert result["margin"] is None + assert result["ranked"][0]["heading"] == "Introduction" + assert result["ranked"][0]["score"] > 0 + assert all(r["score"] == 0 for r in result["ranked"][1:]) + + def test_query_term_present_in_multiple_sections_has_finite_margin(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = ( + "## A\n\nshared shared shared unique_a\n\n" + "## B\n\nshared unique_b\n" + ) + f = _write(tmp_path, content) + result = score_sections(f, "shared") + assert result["unambiguous"] is False + assert result["margin"] is not None + assert result["margin"] > 1.0 # section A repeats "shared" more densely + + def test_no_query_term_matches_anything_has_no_margin(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = score_sections(f, "zzzznomatch") + assert result["margin"] is None + assert result["unambiguous"] is False + assert all(r["score"] == 0 for r in result["ranked"]) + + def test_ranked_is_sorted_descending_by_score(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + result = score_sections(f, "KAPING framework") + scores = [r["score"] for r in result["ranked"]] + assert scores == sorted(scores, reverse=True) + + def test_single_section_with_positive_score_is_unambiguous(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #111: a lone section can't be confused with + anything else -- the same "nothing to compete with" case as a + section that beat every rival's zero score, just with zero rivals + instead of losing ones. Matters for real: route_tier1() only + resolves a Tier-1 agreement when unambiguous, so this previously + forced every single-section document to escalate needlessly.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## Only\n\nSome KAPING content.\n") + result = score_sections(f, "KAPING") + assert len(result["ranked"]) == 1 + assert result["margin"] is None + assert result["unambiguous"] is True + + def test_single_section_with_zero_score_is_not_unambiguous(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "## Only\n\nSome unrelated content.\n") + result = score_sections(f, "zzzznomatch") + assert len(result["ranked"]) == 1 + assert result["margin"] is None + assert result["unambiguous"] is False + + +class TestCmdTfidfScore: + def test_missing_file(self, tmp_path: Path, capsys): + rc = cmd_tfidf_score([str(tmp_path / "nope.md"), "query"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_directory_as_file_argument_is_rejected(self, tmp_path: Path, capsys): + """CodeRabbit PR #110: require_existing_file's .is_file() check + (not .exists()) correctly rejects a directory, but this exact case + was never exercised by a test.""" + rc = cmd_tfidf_score([str(tmp_path), "query"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_non_utf8_file_reports_a_clean_error_not_a_raw_traceback(self, tmp_path: Path, capsys): + f = tmp_path / "bad.md" + f.write_bytes(b"# Title\n\xff\xfe not valid utf-8\n") + rc = cmd_tfidf_score([str(f), "query"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_missing_required_argument_emits_json_error_not_a_plain_text_banner( + self, tmp_path: Path, capsys + ): + """CodeRabbit PR #110: an argparse parsing failure (a required + positional omitted) used to bypass this project's own --json + output contract entirely, printing a plain-text usage banner to + stderr instead of a JSON ERROR result to stdout.""" + f = _write(tmp_path) + rc = cmd_tfidf_score([str(f)]) # query omitted + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_empty_string_query(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + rc = cmd_tfidf_score([str(f), ""]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["margin"] is None + assert out["unambiguous"] is False + assert all(r["score"] == 0 for r in out["ranked"]) + + def test_basic_json_output(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + rc = cmd_tfidf_score([str(f), "KAPING"]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["unambiguous"] is True + assert out["ranked"][0]["heading"] == "Introduction" + + def test_human_output_unambiguous(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_tfidf_score([str(f), "KAPING"]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "unambiguous" in out + assert "Introduction" in out + + def test_human_output_with_margin(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + content = "## A\n\nshared shared shared unique_a\n\n## B\n\nshared unique_b\n" + f = _write(tmp_path, content) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_tfidf_score([str(f), "shared"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "margin" in capsys.readouterr().out + + def test_human_output_no_match(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_tfidf_score([str(f), "zzzznomatch"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "confidence: none" in capsys.readouterr().out + + def test_human_output_headingless_document(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path, "Just a paragraph, no headings.\n") + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_tfidf_score([str(f), "anything"]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "no retrieval sections" in capsys.readouterr().out diff --git a/tests/test_toc.py b/tests/test_toc.py index 4e80f995..de24de8c 100644 --- a/tests/test_toc.py +++ b/tests/test_toc.py @@ -22,6 +22,7 @@ insert_toc_heading, insert_toc_markers, parse_headings, + parse_headings_with_lines, validate_toc, ) @@ -812,6 +813,60 @@ def test_frontmatter_hash_line_is_not_parsed_as_a_heading(self): codes = [w["code"] for w in result["warnings"]] assert "toc-heading-duplicate" not in codes + def test_frontmatter_closed_with_dots_is_still_recognized(self): + """CodeRabbit PR #110 (round 2): YAML permits closing a document + with `...` as well as `---`. Only recognizing `---` meant `...`- + terminated frontmatter was never seen as closed, so every heading + after it (all of them, here) was silently skipped.""" + content = ( + "---\n" + "title: Foo\n" + "...\n" + "# Title\n\n" + "## Section A\n\n" + "## Section B\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [e["code"] for e in result["errors"]] + # A TOC-less document with real headings should trip a missing-TOC + # error -- it can only do that if headings after `...` are seen. + assert "toc-missing" in codes + + def test_indented_terminator_inside_a_block_scalar_is_not_a_real_terminator(self): + """CodeRabbit PR #110 (round 3): an indented `---`/`...` is valid + content *inside* a YAML block scalar (e.g. `description: |`), not + a document terminator -- ending frontmatter early there would let + a later, still-inside-frontmatter `#`-prefixed line get mistaken + for a real Markdown heading.""" + content = ( + "---\n" + "description: |\n" + " ...\n" + "## not a real heading -- still frontmatter content\n" + "title: Foo\n" + "---\n\n" + "# Real Title\n\n" + "## Section A\n" + ).split("\n") + headings = parse_headings_with_lines(content) + assert [text for _level, text, _line in headings] == ["Real Title", "Section A"] + + def test_indented_opening_delimiter_is_not_mistaken_for_frontmatter(self): + """CodeRabbit PR #110 (round 4): an indented ` ---` on the first + line is a valid Markdown indented thematic break, not a YAML + frontmatter opener. Treating it as one would mis-scope everything + up to the next real `---` as frontmatter, skipping any headings in + between.""" + content = ( + " ---\n" + "# Real Title\n\n" + "## Section A\n\n" + "---\n\n" + "## Section B\n" + ).split("\n") + headings = parse_headings_with_lines(content) + assert [text for _level, text, _line in headings] == ["Real Title", "Section A", "Section B"] + def test_depth_jump_warned(self): content = ( "# Title\n\n" @@ -1059,6 +1114,29 @@ def test_real_description_after_regex_tightening_still_suppresses_warning(self): codes = [w["code"] for w in result["warnings"]] assert "toc-missing-description" not in codes + def test_nested_description_field_does_not_suppress_warning(self): + """CodeRabbit PR #110: the regex used to match against the + indentation-stripped line, so a nested key like + `metadata:\n description: text` satisfied the same check as a + root-level `description:` field. Only a root-level field should + count -- a nested one isn't what a caller reading this frontmatter + for a description actually finds.""" + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "---\n" + "metadata:\n" + " description: A description nested under another key.\n" + "---\n\n" + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{filler}\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" in codes + def test_empty_block_scalar_description_still_warns(self): """CodeRabbit PR #109 (second round): `description: |` is a YAML block-scalar marker -- the real content (if any) belongs on @@ -1080,6 +1158,38 @@ def test_empty_block_scalar_description_still_warns(self): codes = [w["code"] for w in result["warnings"]] assert "toc-missing-description" in codes + @pytest.mark.parametrize( + "block_scalar_header", + [ + "description: |2-", + "description: |-2", + "description: | # TODO", + ], + ids=["digit-then-chomp", "chomp-then-digit", "trailing-comment"], + ) + def test_empty_block_scalar_with_valid_indicator_variants_still_warns(self, block_scalar_header): + """CodeRabbit PR #110/#111 (both independently flagged this): YAML + allows the chomping (+/-) and indentation (1-9) indicators in + either order, plus a trailing "# comment" (preceded by whitespace) + -- `|2-`, `|-2`, and `| # TODO` are all valid block-scalar headers + the old regex rejected outright, which made the later logic treat + them as an ordinary (non-empty) scalar value and wrongly suppress + the missing-description warning.""" + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "---\n" + f"{block_scalar_header}\n" + "---\n\n" + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{filler}\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" in codes + def test_populated_block_scalar_description_suppresses_warning(self): """The other side of the block-scalar fix: real indented content under `description: |` must still count as a real description.""" diff --git a/tests/test_tomllib_compat.py b/tests/test_tomllib_compat.py index ff8bd944..a1d75f62 100644 --- a/tests/test_tomllib_compat.py +++ b/tests/test_tomllib_compat.py @@ -1,6 +1,9 @@ import importlib +import os +import subprocess import sys import types +from pathlib import Path import pytest @@ -8,13 +11,24 @@ MODULE_PATH = "studio.utils._tomllib_compat" -def _reload_module(): +def _reload_module(request): # Ensure fresh import sys.modules.pop(MODULE_PATH, None) - return importlib.import_module(MODULE_PATH) - - -def test_stdlib_tomllib(monkeypatch): + module = importlib.import_module(MODULE_PATH) + # This reload executes _tomllib_compat's `import tomllib` line while + # sys.modules["tomllib"] is monkeypatched to a dummy -- the *compat* + # module then caches that dummy as its own `tomllib` attribute. Popping + # it from sys.modules here (after this test's monkeypatch has reverted + # sys.modules["tomllib"] to the real module) forces the next real + # importer to re-resolve against the genuine tomllib, instead of every + # later consumer in this same pytest-xdist worker (toml_utils.py, + # manifest.py, adapter_info.py, ...) silently getting this test's dummy + # forever. + request.addfinalizer(lambda: sys.modules.pop(MODULE_PATH, None)) + return module + + +def test_stdlib_tomllib(monkeypatch, request): dummy = types.ModuleType("tomllib") dummy.DUMMY_FLAG = "stdlib" @@ -22,13 +36,13 @@ def test_stdlib_tomllib(monkeypatch): monkeypatch.setitem(sys.modules, "tomllib", dummy) monkeypatch.setattr(sys, "version_info", (3, 11)) - mod = _reload_module() + mod = _reload_module(request) assert hasattr(mod, "tomllib") assert mod.tomllib is dummy assert mod.__all__ == ["tomllib"] -def test_tomli_fallback(monkeypatch): +def test_tomli_fallback(monkeypatch, request): dummy = types.ModuleType("tomli") dummy.DUMMY_FLAG = "tomli" @@ -36,7 +50,7 @@ def test_tomli_fallback(monkeypatch): monkeypatch.setattr(sys, "version_info", (3, 10)) monkeypatch.setitem(sys.modules, "tomli", dummy) - mod = _reload_module() + mod = _reload_module(request) assert hasattr(mod, "tomllib") # When tomli is used, the compat module aliases it as tomllib assert mod.tomllib is dummy @@ -71,3 +85,49 @@ def _fake_import(name, globals=None, locals=None, fromlist=(), level=0): captured = capsys.readouterr() assert "ERROR: tomllib/tomli not available" in captured.err + +def test_stdlib_reload_does_not_leak_the_dummy_to_a_later_test(tmp_path): + """Regression: _reload_module's fresh import (triggered while + sys.modules["tomllib"] is monkeypatched to a dummy) used to leave that + dummy permanently cached as the compat module's own `tomllib` + attribute in sys.modules, with no cleanup registered anywhere. Any + later, unrelated consumer of `from studio.utils._tomllib_compat import + tomllib` in the same process (toml_utils.py, manifest.py, + adapter_info.py, ...) would then silently get that dummy (no real + `.load()`) forever, instead of the genuine tomllib -- this is exactly + what surfaced as `AttributeError: module 'tomllib' has no attribute + 'load'` in CI once enough test files shifted pytest-xdist's worker + scheduling for this pair to land in the same worker. + + Runs a two-test scenario (mirroring test_stdlib_tomllib, then a "later + consumer") in a real, separate pytest process, so the first test's + finalizer genuinely completes before the second test runs -- calling + _reload_module directly from here couldn't observe its own finalizer. + """ + script = tmp_path / "test_isolated.py" + script.write_text( + "import sys, types, importlib\n" + f"MODULE_PATH = {MODULE_PATH!r}\n" + "\n" + "def test_1_corrupt(monkeypatch, request):\n" + " dummy = types.ModuleType('tomllib')\n" + " monkeypatch.setitem(sys.modules, 'tomllib', dummy)\n" + " monkeypatch.setattr(sys, 'version_info', (3, 11))\n" + " sys.modules.pop(MODULE_PATH, None)\n" + " mod = importlib.import_module(MODULE_PATH)\n" + " assert mod.tomllib is dummy\n" + " request.addfinalizer(lambda: sys.modules.pop(MODULE_PATH, None))\n" + "\n" + "def test_2_consumes_after():\n" + " assert MODULE_PATH not in sys.modules\n", + encoding="utf-8", + ) + repo_root = Path(__file__).resolve().parents[1] + studio_scripts_dir = repo_root / "skills" / "studio" / "scripts" + env = {**os.environ, "PYTHONPATH": str(studio_scripts_dir)} + result = subprocess.run( + [sys.executable, "-m", "pytest", str(script), "-q"], + capture_output=True, text=True, check=False, env=env, + ) + assert result.returncode == 0, result.stdout + result.stderr + diff --git a/tests/test_ui_human_mode.py b/tests/test_ui_human_mode.py index 2ce0cd55..d8933154 100644 --- a/tests/test_ui_human_mode.py +++ b/tests/test_ui_human_mode.py @@ -26,6 +26,7 @@ table, file_action, result, + require_existing_file, _has_color, _c, ui, @@ -287,13 +288,48 @@ def test_result_json_mode(self): set_json_mode(False) +class TestRequireExistingFile(_HumanModeBase): + """Test require_existing_file() — shared by every single-file-argument command.""" + + def test_returns_resolved_path_for_existing_file(self): + with TemporaryDirectory() as tmp: + f = Path(tmp) / "doc.md" + f.write_text("content", encoding="utf-8") + resolved = require_existing_file(str(f)) + self.assertEqual(resolved, f.resolve()) + + def test_returns_none_and_emits_error_for_missing_file(self): + with TemporaryDirectory() as tmp: + missing = Path(tmp) / "nope.md" + buf = io.StringIO() + with redirect_stderr(buf): + resolved = require_existing_file(str(missing)) + self.assertIsNone(resolved) + self.assertIn("File not found", buf.getvalue()) + + def test_json_mode_emits_error_status_payload(self): + set_json_mode(True) + try: + with TemporaryDirectory() as tmp: + missing = Path(tmp) / "nope.md" + buf = io.StringIO() + with redirect_stdout(buf): + resolved = require_existing_file(str(missing)) + self.assertIsNone(resolved) + out = json.loads(buf.getvalue()) + self.assertEqual(out["status"], "ERROR") + self.assertEqual(out["message"], "File not found") + finally: + set_json_mode(False) + + class TestUISingleton(unittest.TestCase): """Test the ui singleton exposes all methods.""" def test_ui_has_all_methods(self): for attr in ["header", "step", "substep", "success", "error", "warn", "info", "detail", "hint", "blank", "divider", "table", - "file_action", "result", "is_json"]: + "file_action", "result", "require_existing_file", "is_json"]: self.assertTrue(hasattr(ui, attr), f"ui missing {attr}") diff --git a/tests/test_usage_report.py b/tests/test_usage_report.py new file mode 100644 index 00000000..61b05014 --- /dev/null +++ b/tests/test_usage_report.py @@ -0,0 +1,109 @@ +"""Tests for the usage-report command (usage_report.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from studio.commands.usage_report import cmd_usage_report +from studio.utils import decision_log as dl + + +class TestCmdUsageReport: + def test_unknown_argument_emits_json_error_not_a_plain_text_banner(self, capsys): + """CodeRabbit PR #111: cmd_usage_report now uses + JsonSafeArgumentParser, so an unrecognized argument must still + emit the project's own --json ERROR contract, not argparse's + default usage banner + SystemExit.""" + rc = cmd_usage_report(["--bogus"]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_no_project_reports_no_log_found(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + rc = cmd_usage_report([]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["summary"]["exists"] is False + assert out["reads"] == {"methods": {}, "total_tokens": 0} + + def test_json_output_reflects_logged_reads(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + dl.record_read("tfidf", "doc.md", 8925, 49676) + dl.record_read("baseline", "doc.md", 8925, 333573) + + rc = cmd_usage_report([]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["reads"]["methods"]["tfidf"]["total_tokens"] == 49676 + assert out["reads"]["total_tokens"] == 49676 + 333573 + assert out["summary"]["exists"] is True + + def test_human_output_no_log(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_usage_report([]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "no decision log found" in capsys.readouterr().out + + def test_human_output_no_reads_yet(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + dl.record("routing", {"a": 1}) # something logged, but no read events + + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_usage_report([]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "no read events logged yet" in capsys.readouterr().out + + def test_human_output_with_reads(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + dl.record_read("tfidf", "doc.md", 8925, 49676) + + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_usage_report([]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "tfidf" in out + assert "49676" in out + + def test_human_output_surfaces_event_counts_and_time_range(self, tmp_path: Path, capsys, monkeypatch): + """CodeRabbit PR #111: the JSON summary includes event_counts/ + first_ts/last_ts, but the human renderer used to print neither -- + an interactive user saw strictly less than a --json caller.""" + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + dl.record_read("tfidf", "doc.md", 8925, 49676) + + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_usage_report([]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "read" in out # event type name from event_counts + assert "time range" in out diff --git a/vulture_whitelist.py b/vulture_whitelist.py index 1c28afcb..dbf3375f 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -16,6 +16,7 @@ from studio.utils.eval_harness import ReferencePresenceScorer, Scenario, ScorerKind, run_suite from studio.utils.eval_judge import Gold from studio.utils.manifest import ManifestLayerState +from studio.utils.okf import write_concept_file is_json = _UI.is_json # staticmethod alias exposed on the ui singleton @@ -47,6 +48,11 @@ annotate_section_summary # noqa: B018 diff_stale_sections # noqa: B018 +# OKF concept-file writer: called by a future external LLM caller after it +# has actually produced a summary, not yet reached from production paths. +# Exercised by tests. See skills/studio/scripts/studio/utils/okf.py. +write_concept_file # noqa: B018 + # cfs map module — symbols retained for layout/configuration completeness. from studio.commands.map.layout import MAX_ROW_W # noqa: E402 from studio.commands.map.categorize import OverrideCategory # noqa: E402 @@ -68,6 +74,7 @@ record_review, record_escalation, record_invocation, + record_read, summarize, ) @@ -81,6 +88,12 @@ record_invocation # noqa: B018 summarize # noqa: B018 +# record_read: called by a future external caller once a read-and-answer step +# actually fires (an agent doing the real read), not yet reached from +# production paths. Exercised by tests. See +# skills/studio/scripts/studio/utils/decision_log.py. +record_read # noqa: B018 + # eval_semantic public API — the semantic-coverage engine. Library + tests only for now; # the `cfs` surface and coverage-report integration are the follow-up, so these are not yet # reached from a production path. Exercised by tests.