Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions architecture/features/traceability-validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ Catches structural and traceability issues that AI agents miss or hallucinate

**Supporting**:
- [x] - `p1` - Imports and module setup for validate-toc command - `inst-toc-imports`
- [x] - `p1` - Validate a single file, never raising: a missing file or a read failure (permission denied, binary/non-UTF-8 content, a TOCTOU race) is reported as its own ERROR result rather than aborting the whole batch and discarding results already collected for earlier files - `inst-toc-validate-one`
- [x] - `p1` - Human-friendly formatter for validate-toc output: a WARN-only file prints its warnings the same way a FAIL file prints its errors, not just the bare status - `inst-toc-format`

### TOC Utilities
Expand Down Expand Up @@ -447,7 +448,7 @@ Catches structural and traceability issues that AI agents miss or hallucinate

**Input**: Markdown file path

**Output**: `cfs doc-index`'s JSON is `{file, cache_hit, total_lines, section_count, sections}`, each `sections[]` entry `{level, heading, line_start, line_end, summary}`. The underlying index dict additionally carries `schema_version`, `path`, and `etag`.
**Output**: `cfs doc-index`'s JSON is `{file, cache_hit, total_lines, section_count, sections, section_level, retrieval_section_count, retrieval_sections}` -- every heading's own line range in `sections[]` (`{level, heading, line_start, line_end, summary}`), plus a coarser "one chunk per real section" grouping in `retrieval_sections[]` at an inferred heading level, each with a content hash and a summary slot. The underlying cached index dict additionally carries `schema_version`, `path`, and `etag`.

A cached, read-once-per-file structural index for Markdown JIT retrieval (see
constructorfabric/studio#104): parsing a file's headings/section boundaries
Expand All @@ -456,19 +457,27 @@ size) changes.
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.
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.

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`
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`

**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`
- [x] - `p1` - Resolve the cache file location within the Studio directory owning the indexed file, resolved from the file's own path (not the process's working directory) - `inst-doc-index-cache-path`
- [x] - `p1` - `cfs doc-index` CLI wrapper: parse arguments, build the JSON output payload, reporting a clean error for a missing or unreadable file - `inst-doc-index-cmd`
- [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`

### Markdown Parsing Utilities

Expand Down
16 changes: 15 additions & 1 deletion skills/studio/scripts/studio/commands/doc_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ def cmd_doc_index(argv: List[str]) -> int:
prog="cfs doc-index",
description=(
"Build or reuse a cached heading/section index for a Markdown file, "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cfs doc-index --help never explains how section_level is inferred

Severity: Minor

Problem
cfs doc-index picks section_level via a frequency-based heuristic in infer_section_level(): the heading level used most often wins, levels appearing exactly once are excluded as candidates, and ties (or an all-singletons document) fall back to the shallowest level. This choice directly determines which lines get grouped into retrieval_sections in both JSON and human-readable output. None of this is mentioned anywhere a CLI user would see it.

How to reproduce
Run cfs doc-index --help (or read the argparse.ArgumentParser description in commands/doc_index.py). The description reads only: "Build or reuse a cached heading/section index for a Markdown file, so navigation reads the file's structure once, not once per query." No mention of section_level, how it's chosen, or that it can differ from the "obvious" chapter level in irregularly-leveled documents (the exact PDF-conversion scenario infer_section_level()'s own docstring cites as its motivating case).

Expected behavior
A user seeing "section_level": 5 (say) instead of the level they expected should have a way, via --help or the command's own output, to understand that the level was inferred from heading-frequency, not simply "H1/H2 is always the section level."

Actual behavior
--help gives no indication that section-level selection is heuristic at all.

Impact
Minor UX/discoverability gap — doesn't cause incorrect behavior, but makes an already-nontrivial, silently-applied heuristic harder to audit or trust from the CLI alone.

Suggested correction
Extend the argparse.ArgumentParser(description=...) in cmd_doc_index() (or add a short note to _human_doc_index()'s output) with one sentence, e.g.: "section_level is inferred from the most frequently repeated heading level (ties prefer the shallower level); a level used only once is never chosen."

How to verify
Run cfs doc-index --help after the fix and confirm the heuristic is described.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8b80cd1: extended cmd_doc_index's argparse description with a one-sentence explanation of the heuristic (most-frequent heading level wins; a level used only once is never chosen). Regression test: test_help_explains_how_section_level_is_inferred.

"so navigation reads the file's structure once, not once per query."
"so navigation reads the file's structure once, not once per query. "
"section_level is inferred from the most frequently repeated heading "
"level (ties prefer the shallower level); a level used only once is "
"never chosen."
),
)
p.add_argument("file", help="Markdown file path")
Expand Down Expand Up @@ -58,6 +61,9 @@ def cmd_doc_index(argv: List[str]) -> int:
"total_lines": index["total_lines"],
"section_count": len(index["sections"]),
"sections": index["sections"],
"section_level": index["section_level"],
"retrieval_section_count": len(index["retrieval_sections"]),
"retrieval_sections": index["retrieval_sections"],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
ui.result(output, human_fn=_human_doc_index)
return 0
Expand All @@ -75,4 +81,12 @@ def _human_doc_index(data: dict) -> None:
summary = f" — {s['summary']}" if s.get("summary") else ""
ui.substep(f" H{s['level']} [{s['line_start']}-{s['line_end']}] {s['heading']}{summary}")
ui.blank()

level = data["section_level"]
ui.step(f"Retrieval sections (level {level}, {data['retrieval_section_count']} section(s))" if level is not 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}")
ui.blank()
# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd-format
95 changes: 56 additions & 39 deletions skills/studio/scripts/studio/commands/validate_toc.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,58 @@
from pathlib import Path
from typing import List

from ..utils import error_codes as EC
from ..utils.toc import DEFAULT_MAX_SECTION_LINES, add_toc_max_level_argument, validate_toc
from ..utils.ui import ui
# @cpt-end:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-imports

# @cpt-begin:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-validate-one
def _validate_one_file(filepath: Path, args: argparse.Namespace) -> dict:
"""Validate a single file, returning its result dict. Never raises --
a missing file or a read failure (permission denied, binary/non-UTF-8
content, a TOCTOU race) is reported as an ERROR result instead, so one
bad file in a batch can't abort validation of the rest.
"""
if not filepath.is_file():
return {
"file": str(filepath),
"status": "ERROR",
"message": "File not found",
"code": EC.FILE_LOAD_ERROR,
}

try:
content = filepath.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError) as exc:
return {
"file": str(filepath),
"status": "ERROR",
"message": f"Could not read file: {exc}",
"code": EC.FILE_READ_ERROR,
}

report = validate_toc(
content,
artifact_path=filepath,
max_heading_level=args.max_level,
max_section_lines=args.max_section_lines,
)
errors = report.get("errors", [])
warnings = report.get("warnings", [])
file_result: dict = {
"file": str(filepath),
"status": "FAIL" if errors else ("WARN" if warnings else "PASS"),
"error_count": len(errors),
"warning_count": len(warnings),
}
if args.verbose or errors:
file_result["errors"] = errors
if args.verbose or warnings:
file_result["warnings"] = warnings
return file_result
# @cpt-end:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-validate-one


def cmd_validate_toc(argv: List[str]) -> int:
"""Validate Table of Contents in markdown files."""
# @cpt-begin:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-parse-args
Expand Down Expand Up @@ -46,50 +94,19 @@ def cmd_validate_toc(argv: List[str]) -> int:
# @cpt-end:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-parse-args

# @cpt-begin:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-resolve-files
results = []
total_errors = 0
total_warnings = 0
files_to_validate = [Path(f).resolve() for f in args.files]
# @cpt-end:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-resolve-files

# @cpt-begin:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-foreach-file
for filepath in files_to_validate:

if not filepath.is_file():
results.append({
"file": str(filepath),
"status": "ERROR",
"message": "File not found",
})
results = [_validate_one_file(filepath, args) for filepath in files_to_validate]
total_errors = 0
total_warnings = 0
for file_result in results:
if file_result["status"] == "ERROR":
total_errors += 1
continue

content = filepath.read_text(encoding="utf-8")
report = validate_toc(
content,
artifact_path=filepath,
max_heading_level=args.max_level,
max_section_lines=args.max_section_lines,
)

errors = report.get("errors", [])
warnings = report.get("warnings", [])
total_errors += len(errors)
total_warnings += len(warnings)

file_result: dict = {
"file": str(filepath),
"status": "FAIL" if errors else ("WARN" if warnings else "PASS"),
"error_count": len(errors),
"warning_count": len(warnings),
}

if args.verbose or errors:
file_result["errors"] = errors
if args.verbose or warnings:
file_result["warnings"] = warnings

results.append(file_result)
else:
total_errors += file_result["error_count"]
total_warnings += file_result["warning_count"]
# @cpt-end:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-foreach-file

# @cpt-begin:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-return
Expand Down
Loading
Loading