feat: doc-to-code linkage manifest for deterministic selection - #69
feat: doc-to-code linkage manifest for deterministic selection#69Benkapner wants to merge 3 commits into
Conversation
File selection is currently an LLM guess on every run. Where a maintainer declares which source files a doc covers, selection should be deterministic, free, and correct. Add a parser for YAML front-matter in .md, comment directives in .rst, and comment directives in .adoc. Parser only; no selection changes yet.
Any doc whose declared covers paths intersect the diff is included with no LLM call for that decision. The LLM selection pass still runs for undeclared docs. This is additive, not a replacement.
Add a README section showing front-matter syntax for all three formats and positioning linkage as an incremental adoption path.
|
🤖 Finished Review · ✅ Success · Started 5:49 AM UTC · Completed 6:08 AM UTC Commit: |
ReviewFindingsHigh
Medium
Low
Labels: PR adds a new feature (doc-to-code linkage) with Python implementation Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| import re | ||
| from pathlib import Path | ||
|
|
||
| import yaml |
There was a problem hiding this comment.
[high] missing-dependency
linkage.py imports yaml (PyYAML) but pyyaml is not declared in pyproject.toml production dependencies. PyYAML is only available as a transitive dependency of pre-commit (dev-only). The Dockerfile installs only main dependencies, so this will cause an ImportError at runtime when find_declared_docs is called for .md files.
Suggested fix: Add pyyaml to the dependencies list in pyproject.toml.
| # Merge in declared docs (front-matter linkage). These skip the LLM call. | ||
| from linkage import find_declared_docs | ||
|
|
||
| declared_docs = find_declared_docs(diff) |
There was a problem hiding this comment.
[medium] logic-error
The declared docs merge runs unconditionally after file discovery, including when relevant_files was set from a previous interactive review's accepted files. If a user ran [review-docs], rejected a doc file with a covers front-matter declaration, then ran [update-docs], the merge re-adds the rejected file — overriding the user's explicit rejection and breaking the [update-docs] contract documented in CLAUDE.md.
Suggested fix: Filter rejected files from declared_paths before merging, or skip linkage merge when honoring a previous review.
| Returns a list of (doc_path, "declared") tuples for docs that match, | ||
| and scans all doc files in doc_root. | ||
| """ | ||
| changed = extract_changed_paths(diff_text) |
There was a problem hiding this comment.
[medium] symlink-following
find_declared_docs uses Path(doc_root).rglob('*') which follows symlinks by default. In the GitHub Actions context where untrusted PRs are processed, a PR could introduce a symlink pointing outside the doc root; file contents would be read by parse_doc_frontmatter.
Suggested fix: Add if doc.is_symlink(): continue before processing each path.
| def parse_doc_frontmatter(file_path): | ||
| """Extract code-to-docs front-matter from a documentation file. | ||
|
|
||
| Supports YAML front-matter (--- delimiters) for .md files, and |
There was a problem hiding this comment.
[medium] path-traversal
parse_doc_frontmatter reads files via Path(file_path).read_text() without calling validate_file_path() from security_utils.py, which is used for all file I/O in generation.py. While paths come from rglob (not direct user input), the inconsistency with the codebase's existing security pattern is a gap.
Suggested fix: Add validate_file_path(file_path, base_dir=doc_root) before reading, consistent with generation.py.
| for doc in Path(doc_root).rglob("*"): | ||
| if not doc.is_file() or doc.suffix not in doc_extensions: | ||
| continue | ||
| if ".doc-index" in str(doc): |
There was a problem hiding this comment.
[low] path-traversal
covers paths in front-matter are attacker-controlled values with no validation for .., absolute paths, or null bytes. Currently only used in string matching (no I/O), but lacks defense-in-depth.
|
|
||
| Returns a list of (doc_path, "declared") tuples for docs that match, | ||
| and scans all doc files in doc_root. | ||
| """ |
There was a problem hiding this comment.
[low] return-type-consistency
find_declared_docs returns tagged tuples (str, 'declared') but the call site immediately discards the tag. No other discovery function uses this pattern.
| @@ -0,0 +1,107 @@ | |||
| """Doc-to-code linkage: deterministic file selection from front-matter declarations.""" | |||
There was a problem hiding this comment.
[low] module-docstring
Single-line module docstring where most source modules use multi-line format with an elaborating paragraph.
| def extract_changed_paths(diff_text): | ||
| """Extract all file paths changed in a unified diff.""" | ||
| paths = set() | ||
| for match in re.finditer(r"^diff --git a/(.+?) b/", diff_text, re.MULTILINE): |
There was a problem hiding this comment.
[low] function-docstring
extract_changed_paths and find_declared_docs lack Args: and Returns: sections used by other public functions in the codebase.
|
|
||
| import re | ||
| from pathlib import Path | ||
|
|
There was a problem hiding this comment.
[low] import-style
Missing blank line between stdlib and third-party import groups.
| return {} | ||
| parts = content.split("---", 2) | ||
| if len(parts) < 3: | ||
| return {} |
There was a problem hiding this comment.
[low] edge-case
Split-based front-matter delimiter parsing has standard limitations with consecutive --- sequences. No data corruption; declarations silently ignored in edge cases.
fullsend-ai-review
Bot
added
feature
python
1 participant
Summary
Adds front-matter declarations so doc files can declare which source files they cover. Declared docs are selected deterministically with no LLM call.
src/linkage.py): reads YAML front-matter (.md), directive comments (.rst), and line comments (.adoc)Test plan
uv run pytest -vpasses (428 tests)