Feature/style config

[csoceanu]

Summary

This PR adds three capabilities to code-to-docs:

1. Persistent style guidelines

Users can define documentation style rules in .code-to-docs/style.md (auto-detected) or via the style-config-path action input. Guidelines are injected into generation prompts with clear precedence: per-file instructions > user instructions > style guidelines > base rules.

2. Output format validation with retry

LLM output is validated using real parsers (markdown, docutils, asciidoctor.js) before writing to files. If validation fails, the output is sent back to the LLM with the error details for correction (max 2 retries). Files with persistent format errors are skipped entirely — no invalid output makes it into a PR.

3. Discovery pipeline improvements

During testing, several issues with the file discovery pipeline were identified and fixed:

• Sub-folder indexing: indexes are now built per sub-folder instead of per top-level folder, producing more precise candidate sets
• Single-step file selection: the LLM picks specific files directly from index descriptions in one step, replacing the previous two-stage area-file selection that required loading all candidate files
• Root-level docs: indexed under a virtual _root folder instead of being unconditionally included in every query
• Folder name mapping bug: load_all_indexes() was converting hyphens to slashes, causing sub-folder files to be missed
• Same-repo update-docs: doc changes are committed directly to the code PR branch instead of creating a separate docs PR

Other fixes

• Strip code fences from LLM output
• Ensure trailing newline on generated files
• Validate returned file paths exist on disk (reject hallucinated paths)
• Reject glob patterns in LLM-returned filenames
• Cap validation error output to prevent prompt injection via asciidoctor stderr
• Asciidoctor runs in secure mode (disables include:: directives)
• Warn when GH_TOKEN or PR_HEAD_SHA is missing in same-repo push

Test plan

• Unit tests: 239 passing
• Integration test: tested on a live repo with [review-docs] and [update-docs] commands
• Style guidelines loaded and applied to generated documentation
• Format validation catches and strips code fences
• Same-repo [update-docs] commits directly to the PR branch
• Sub-folder indexing produces correct candidate counts

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:40 PM UTC · Completed 2:53 PM UTC
Commit: 06f62db · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [logic-error] src/suggest_docs.py:338 — The same-repo push uses PR_HEAD_SHA as the git push refspec target (HEAD:{pr_head_ref}), but the action input is named pr-head-sha with description "Head reference/SHA for the PR." If a user passes an actual commit SHA rather than a branch name, git push to a SHA ref will fail. The README examples correctly pass head_ref (a branch name), but the naming is misleading.
  Remediation: Rename the input to pr-head-ref to clarify it should be a branch name, or add validation to detect SHA vs branch.

• [logic-error] src/generation.py:316 — Style guidelines budget uses len(diff) (the full, untruncated diff) to estimate remaining space. If the diff is large, style_budget goes negative and max(0, style_budget) drops style guidelines entirely, even though the diff will be truncated later to fit. Style guidelines are unnecessarily sacrificed.
  Remediation: Compute style budget based on a reserved allocation, or restructure the budget calculation to account for the fact that the diff will be truncated.

• [prompt-injection] src/generation.py:316 — Style guidelines from repository files are interpolated directly into the LLM prompt. The delimiters (<<<STYLE_GUIDELINES/>>>END_STYLE_GUIDELINES) and DATA BLOCK instruction are soft defenses. A malicious contributor could craft .code-to-docs/style.md with prompt injection payloads that override system instructions.
  Remediation: Document that the style config file should be treated as trusted content and protected via CODEOWNERS.

• [scope-creep] — This PR bundles three independent capabilities (style guidelines, output format validation, discovery pipeline changes) into a single change without a linked issue. The PR body is transparent about all three features, but the breadth makes review harder and increases merge risk.
  Remediation: Consider splitting into separate PRs or creating a tracking issue.

• [scope-creep] src/doc_index.py:88 — The discovery pipeline is fundamentally changed from top-level folder indexing to immediate-parent folder indexing, and from a two-stage approach (find areas, then select files) to single-stage file selection. Tests are updated consistently, but this is a significant behavioral change.
  Remediation: Document the rationale for the granularity change.

• [architectural-divergence] src/discovery.py:6 — The module docstring still references "an optimized two-stage discovery pipeline" but the PR changes this to a single-stage approach.
  Remediation: Update the module-level docstring to reflect the new single-stage architecture.

• [scope-creep] src/suggest_docs.py:325 — Same-repo workflow changed from creating a separate branch/PR to committing directly to the PR head branch. Both approaches are valid, but this is a meaningful workflow change bundled with other features.
  Remediation: Document the rationale for this change.

Low

• [stale-reference] src/doc_index.py:31 — _process_file_selection_batch does not filter LLM results against known batch folders (unlike the old _process_area_batch). Post-hoc validation in find_relevant_files_from_indexes checks file existence and extensions, which mitigates the risk.

• [edge-case] src/doc_index.py:533 — load_all_indexes stem-to-folder mapping could collide if a repo has both guides/operations (nested) and guides-operations (hyphenated) folders. Extremely unlikely in practice.

• [test-integrity] tests/test_generation.py:87 — Test assertions updated for trailing newlines are consistent with the new behavior. A test verifying no double-newline when output already ends with \n would improve coverage.

• [prompt-injection] src/generation.py:360 — Format-fix retry loop feeds LLM output back into a new prompt. This is a standard self-correction pattern; the risk is theoretical.

• [path-traversal] src/config.py:131 — load_style_config uses validate_file_path() with Path.cwd() as base. If cwd changes later (via os.chdir), the validation base shifts. Not currently exploitable since load_style_config is called before any chdir.

• [docstring-style] src/doc_index.py:88 — Docstrings for get_doc_folders and get_docs_in_folder no longer match the changed behavior (sub-folder granularity, non-recursive glob). Should be updated.

• [scope-tier-mismatch] README.md:177 — Removed file summaries from performance section, but the summaries infrastructure still exists in doc_index.py. Clarify whether file summaries are deprecated.

• [dead-code] src/generation.py:415 — return output after the retry for-loop is unreachable.

• [edge-case] src/doc_index.py:462 — Filter now checks parent.parts for underscore/dot prefixes but no longer checks filenames. Files like _sidebar.md would be included.

• [error-handling] src/generation.py:389 — Fix retry exception discards content silently. Logging a snippet of the discarded output would aid debugging.

Previous run

Review

Findings

High

• [stale-feature-description] README.md:177 — The "Performance Optimization" section describes a "two-stage caching system" with "File Summaries — Cached summaries of long documentation files, reused across runs." This PR refactors discovery from two-stage (areas → files with summaries) to single-stage (direct file selection from indexes with per-file descriptions). The README section is now factually incorrect.
  Remediation: Update the "Performance Optimization" section to reflect the current single-stage approach using per-file descriptions in folder indexes. Remove the "File Summaries" bullet.

Medium

• [logic-error] src/doc_index.py — The get_doc_folders filter any(part.startswith('.') or part.startswith('_') for part in rel_path.parts) checks ALL path parts including the filename. Documentation files named _index.md (common in Hugo-based doc sites) would be silently excluded even if they're in a normal directory. The old code only checked the top-level folder name.
  Remediation: Exclude only directory parts from the underscore check: if any(part.startswith('.') or part.startswith('_') for part in rel_path.parent.parts):.

• [logic-error] src/doc_index.py — The old _process_area_batch filtered LLM results to only include folders from the current batch. The new _process_file_selection_batch removes this filter entirely. The downstream validation checks file existence and extension but does not verify the file belongs to a folder in the current batch, allowing hallucinated paths matching real files from other batches to be incorrectly included.
  Remediation: Add validation that returned file paths correspond to files documented in the batch's folder indexes.

• [command-injection] src/generation.py:949 — The _validate_asciidoc function passes LLM-generated output to asciidoctor via stdin. Asciidoctor processes include:: directives that can read arbitrary files from disk. An LLM output containing include::/etc/passwd[] would cause asciidoctor to read those files. While capture_output=True prevents leaking to external parties, file contents could appear in action logs via stderr.
  Remediation: Pass --safe-mode=secure (or -S secure) to the asciidoctor invocation. Secure mode disables include directives and file system access.

• [scope-creep] src/doc_index.py — This PR bundles two distinct architectural changes under a single "style config" feature: (1) persistent style guidelines, and (2) a complete refactoring of documentation discovery from 2-stage area-based to 1-stage file-based indexing (~300 lines in doc_index.py + discovery.py). The indexing refactoring is not mentioned in the PR title or description.
  Remediation: Split into two PRs, or document the dependency and justification in the PR description.

• [undocumented-architecture-change] src/doc_index.py:406 — The PR changes indexing from top-level folder granularity (guides/, tutorials/) to immediate-parent folder granularity (guides/operations/, guides/configuration/) and introduces ROOT_LEVEL_FOLDER. No PR description or design documentation explains the rationale, migration path, or backward compatibility considerations.
  Remediation: Document this architectural change in the PR description. Explain the rationale and how existing deployments with old-style indexes will behave.

Low

• [logic-error] src/generation.py — The trailing return output after the retry for loop is dead code. Also, the newline-appending (output += '\n') before the loop is not re-applied after the fix-prompt retry replaces output via strip_code_fences.
• [logic-error] src/generation.py — style_budget uses len(diff) (original diff length) instead of the truncated diff, making the budget overly pessimistic. When budget reaches 0, a useless truncation marker is injected.
• [stale-reference] src/doc_index.py — find_relevant_areas_from_indexes, _process_area_batch, and get_files_in_areas remain defined but are no longer imported — dead code.
• [logic-error] src/doc_index.py — batch_indexes string concatenation: "="*50 + "\n\n".join(...) puts separator = signs only before the first item due to operator precedence (carried-forward bug).
• [edge-case] src/generation.py — _validate_markdown uses Python's markdown library which never raises on malformed input, making validation a no-op for .md files.
• [prompt-injection] src/generation.py:317 — Style guidelines are injected directly into LLM prompts. The delimiter markers are not robust against adversarial content, though the threat model is limited since the file must be committed by someone with write access.
• [function-documentation] src/generation.py — ask_ai_for_updated_content lacks docstring for the new style_guidelines parameter.
• [function-documentation] src/config.py:147 — load_style_config has a single-line docstring inconsistent with other config functions that provide Args/Returns sections.

Info

• [permission-changes] action.yml:81 — New action input style-config-path added and mapped to STYLE_CONFIG_PATH env var. Security controls provide adequate guardrails.
• [test-coverage] tests/test_style_config.py — Good test coverage for style config, but limited tests for indexing refactoring migration/backward compatibility.

Previous run (2)

Review

Reason: stale-head

The review agent reviewed commit 2d21d542135c4ee34a744d541539757c5c841339 but the PR HEAD is now 4fabdf16ddd08d3101dd7e1332f3e05121a72213. This review was discarded to avoid approving unreviewed code.

Previous run (3)

Review

Findings

High

• [logic-error] src/suggest_docs.py:1179 — PR_HEAD_SHA is used as the push refspec target (git push origin HEAD:{pr_head}), but PR_HEAD_SHA is a commit SHA, not a branch name. Pushing to a SHA refspec will fail or create a detached ref on the remote — it cannot update the PR branch. To push commits onto the PR's branch, the code needs the branch name (e.g., github.event.pull_request.head.ref), not the SHA.
  Remediation: Use the PR head branch name (e.g., add a pr-head-ref input or derive the branch name from the GitHub API) instead of the SHA.

Medium

• [logic-error] src/suggest_docs.py:1176 — When GH_TOKEN is not set, the same-repo path commits locally but never pushes. No warning or error is emitted, so doc updates are silently lost in the ephemeral container.
  Remediation: Add an else branch that logs a warning or treat this as a fatal error.

• [edge-case] src/generation.py:1031 — The style_budget calculation does not account for the diff content that will be inserted into {DIFF_PLACEHOLDER}. The diff can be very large, so style_budget could be overestimated. While truncate_diff later computes from the updated prompt_template length (which includes style), the initial budget allocation for style is suboptimal and could crowd out diff content.
  Remediation: Compute the style budget after accounting for a reasonable diff size estimate, or restructure the budget computation to allocate shares.

• [logic-error] src/doc_index.py:982 — _process_file_selection_batch no longer filters LLM-returned file paths against the batch's known folders. The old _process_area_batch had a guard (relevant_areas = [f for f in relevant_areas if f in batch_folders]). The post-hoc filter checks for glob patterns and doc extensions but does not verify returned paths exist in any index. LLM-hallucinated paths would pass through and fail silently downstream.
  Remediation: Add validation that checks returned file paths against actual files known from the indexes, or verify file existence before passing to downstream processing.

• [logic-error] src/generation.py:1072 — In the validation retry loop, if the LLM fix call raises a non-context exception, the code breaks out and falls through to return output — returning content that failed validation. The else branch (which returns NO_UPDATE_NEEDED) only executes when attempt == MAX_FORMAT_RETRIES, not on exception-induced breaks.
  Remediation: After the break on exception, return NO_UPDATE_NEEDED instead of falling through.

Low

• [logic-error] src/generation.py:893 — _validate_markdown calls markdown.markdown(text), but the markdown library is intentionally lenient and will parse virtually any input without raising exceptions. Validation is effectively a no-op for .md files.

• [edge-case] src/generation.py:860 — strip_code_fences regex requires a newline before the closing triple-backtick. If the LLM returns fences without a preceding newline, stripping won't occur.

• [test-inadequate] tests/test_style_config.py — test_retries_on_invalid_format_then_succeeds never exercises the retry path because strip_code_fences runs before validate_format, so the first attempt succeeds. The test comment acknowledges this.

• [injection] src/generation.py:311 — Style guidelines from a repository .md file are interpolated into the LLM prompt. While mitigated by DATA BLOCK framing, instruction hierarchy, and auto-detect requiring repo write access, delimiter-based containment is not a reliable security boundary.

• [injection] src/generation.py:131 — _validate_asciidoc stderr output from asciidoctor could echo attacker-controlled content back into a retry prompt. Multi-step attack chain makes this low risk.

• [incomplete-doc] README.md:113 — The workflow YAML example does not include the new style-config-path action input. The parameter is optional with auto-detection, so this is minor.

• [misplaced-abstraction] src/config.py — load_style_config imports from security_utils, adding a dependency from the foundational config module. No actual circular dependency risk exists (security_utils is a leaf module), but the placement is suboptimal.

Info

• [scope-creep] src/doc_index.py — PR titled "Feature/style config" includes a major discovery pipeline refactoring (two-stage to one-stage lookup, sub-folder granularity). These changes appear interdependent with the style config feature but could benefit from explicit justification.

• [scope-creep] src/generation.py — Format validation with retry loop and new Dockerfile dependencies (asciidoctor, markdown, docutils) are complementary to style guidelines but represent a separate feature.

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

[medium] logic-error

_process_file_selection_batch no longer filters LLM-returned file paths against the batch's known folders. LLM-hallucinated paths would pass through the post-hoc filter and fail silently downstream.

Suggested fix: Add validation that checks returned file paths against actual files known from the indexes.

[fullsend-ai-review]

[low] injection

Style guidelines from a repository .md file are interpolated into the LLM prompt with delimiter-based containment. While mitigated by instruction hierarchy and requiring repo write access, delimiter containment is not a reliable security boundary.

[fullsend-ai-review]

[low] injection

_validate_asciidoc stderr output could echo attacker-controlled content back into a retry prompt. Multi-step attack chain makes this low risk.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:08 PM UTC · Completed 3:26 PM UTC
Commit: 06f62db · View workflow run →

[fullsend-ai-review]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:28 PM UTC · Completed 3:41 PM UTC
Commit: 06f62db · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:55 PM UTC · Completed 4:10 PM UTC
Commit: 06f62db · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:14 PM UTC · Completed 4:30 PM UTC
Commit: 06f62db · View workflow run →

[fullsend-ai-review]

Review skipped — this PR is already merged.

The /fs-review command only reviews open pull requests.

_{Posted by fullsend post-review check}