qa-reviewer

# QA Reviewer (per-section, phase C) Deterministic + LLM-assisted gate that fires immediately after [`section-doc-writer`](../section-doc-writer/SKILL.md) writes a per-section markdown. This skill owns the **mechanical** half — a Python checker that enforces the binding authoring rules from the templates and profile. The semantic prose review (does the Purpose paragraph actually describe the business intent? do the citations point at the right lines?) is the LLM `qa-reviewer` agent's job and consumes the same `findings[]` sidecar as its starting point. ## When to Use - Immediately after every `section-doc-writer` per-section invocation (the pipeline-DAG order). - Re-validating an existing `docs/<lang>/<file>/sections/*.md` after manual edits. - Pre-commit / CI gate before phase-D diagram rendering or phase-N portal build. - The user asks to "review", "lint", "qa-check", "audit citations" on a section doc. Do **not** use this skill for: - chunking ([`cobol-chunking`](../cobol-chunking/SKILL.md)), - facts extraction ([`cobol-facts-extraction`](../cobol-facts-extraction/SKILL.md)), - per-chunk text / facts prep ([`chunk-text-extraction`](../chunk-text-extraction/SKILL.md), [`facts-slicing`](../facts-slicing/SKILL.md)), - writing the section markdown ([`section-doc-writer`](../section-doc-writer/SKILL.md)), - per-file `index.md` / `complete.md` review or diagram-renderer review — those are different `qa-reviewer` scopes in `pipeline-dag.yaml` phase C. ## Output Contract For section markdown `docs/<lang>/<basename-with-ext>/sections/<chunk_id_sanitized>.md` reviewed at language `<lang>`: ``` docs/_reviews/section-doc/<basename-with-ext>__<chunk_id_sanitized>__<lang>.json ``` Shape: ```jsonc { "review_version": "1.0", "section_doc": "docs/en/DEMO100.CBL/sections/…__B200-EX.md", "section_doc_sha": "<64-hex>", // hash of reviewed bytes "source_path": "repos/sample1/COBOL/DEMO100.CBL", "chunk_id": "DEMO100/procedure-division/B200-CONTROLLO-PARAMETRO/B200-EX", "language": "en", "profile_path": "config/templates/section-doc.profile.yaml", "thresholds": { "min_total_words": 1500, "min_purpose_sentences": 4, "min_purpose_words": 80, "max_tldr_words": 30, "identifier_lock_regex": "[A-Z][A-Z0-9-]*(\\.[A-Z0-9-]+)?", "require_citations": true }, "ok": false, "findings": [ { "code": "TLDR_FORBIDDEN_TOKEN", "severity": "error", "section": "Summary", "line": 12, "message": "Summary contains forbidden COBOL jargon: 'PERFORM'." } /* zero or more; sorted (severity desc, line asc) */ ], "counters": { "total_words": 842, "tldr_words": 22, "purpose_sentences": 5, "purpose_words": 112, "src_footnotes": 7, "inline_file_tags": 0 } } ``` Exit codes mirror the verdict: - `0` — `ok=true`, zero `severity=error` findings. - `1` — at least one `severity=error` finding (review file still written). - `4` — required input missing (markdown not found or bundle unavailable). - `5` — stale bundle (`bundle.chunk_text_sha` mismatch vs. on-disk chunk text). `severity` is one of `error` (blocks pipeline), `warn` (logged, non-blocking), `info` (advisory). ## Mechanical Findings (enforced by the checker) | Code | What triggers it | Severity | |-------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|----------| | `TLDR_MISSING` | No `## Summary` block (or legacy `## TL;DR`), or block has zero non-whitespace content. | error | | `TLDR_TOO_LONG` | Summary exceeds `thresholds.max_tldr_words` (default 30). | error | | `TLDR_MULTI_SENTENCE` | Summary contains more than one sentence (split on `.`, `!`, `?`). | error | | `TLDR_FORBIDDEN_TOKEN` | Summary contains any of: `PERFORM`, `EVALUATE`, `WORKING-STORAGE`, `GO TO`, `DECLARATIVES`, or a line/verb count phrase. | error | | `PURPOSE_MISSING` | No `## Purpose` block. | error | | `PURPOSE_TOO_SHORT_SENTENCES` | Purpose has fewer than `min_purpose_sentences` (default 4) sentences. | error | | `PURPOSE_TOO_SHORT_WORDS` | Purpose has fewer than `min_purpose_words` (default 80) words. | error | | `PURPOSE_FORBIDDEN_OPENER` | Purpose starts with one of: `"This section contains the following…"`, `"The section spans lines"`, `"This PROCEDURE DIVISION section implements the following logic…"`, or equivalent transcript-style preface. | error | | `PURPOSE_LINE_COUNT_PROSE` | Purpose mentions a line count, line range, or verb count (`PERFORM`, `CALL`, `GO TO` counts). | error | | `INLINE_FILE_TAG` | Any occurrence of `[FILE:Lx-Ly]` style citation in body (must use `[^src-N]` footnotes). | error | | `MISSING_SOURCES_BLOCK` | `require_citations=true` and document has no `## Sources` heading. | error | | `MISSING_SRC_FOOTNOTES` | `require_citations=true` and document has zero `[^src-N]` footnote references. | error | | `IDENTIFIER_NOT_BACKTICKED` | A token matching `identifier_lock_regex` appears in prose but is not wrapped in backticks. | warn | | `BANNED_BANNER` | The reviewed doc contains the forbidden "Self-contained document. Bundles… from facts.json…" banner. | error | | `MIN_TOTAL_WORDS` | Total word count below `thresholds.min_total_words` (default 1500). At per-section scope this is `warn` (the threshold is a per-document aggregate); at per-file scope it is `error`. | warn | | `MISSING_DIAGRAM` | Companion `docs/_shared/<file>/diagrams/section-<chunk_id>.mmd` is missing. | warn | | `COPYBOOK_INCLUSION_SIDECAR_DRIFT` | Copybook chunk: inline ` ```mermaid ` block under `## Inclusion sites → Inclusion graph` is not byte-identical to `bundle.output_paths.diagram`. Sidecar must equal the inline source per `copybook.template.md` "DIAGRAM OWNERSHIP". | error | | `COPYBOOK_INCLUSION_SIDECAR_UNEXPECTED` | Copybook chunk: sidecar `.mmd` exists but the inclusion-graph gate is not met (< 2 includers in `incoming_xref[via=copy]` AND no `REPLACING` divergence). Sidecar must be pruned. | warn | | `COPYBOOK_STRUCTURE_IN_SIDECAR` | Copybook chunk: sidecar `.mmd` begins with `classDiagram`, `erDiagram`, or otherwise matches the structure-diagram contract instead of `flowchart LR`. The structure diagram is NEVER written to the sidecar (owned by `diagram-renderer`'s `er-fragment.mmd`). | error | All checks are pure regex / counting — no LLM call. The LLM `qa-reviewer` agent reads this JSON and produces additional `severity` entries with codes prefixed `SEMANTIC_…` (e.g. `SEMANTIC_PURPOSE_OFF_TOPIC`). ## Procedure Review exactly the requested `bundle.languages` at `bundle.output_paths`, passing the selected language explicitly to the checker. Do not require an English document when English was not requested. Cross-language structural parity applies only when multiple languages are requested; shared neutral diagrams remain shared. 1. **Resolve inputs.** Either pass the markdown path directly, or pass `--bundle <path>` (preferred) and read the markdown path from `bundle.output_paths`. Two sibling artifacts: - `docs/_shared/<basename-with-ext>/_bundles/<chunk_id_sanitized>.bundle.json` — **authoritative**; consumed by the mechanical checker `check-section-doc.py` (needs structured access to `chunk_text_sha`, `facts_slice`, `output_paths`, `profile`). - `docs/_shared/<basename-with-ext>/_bundles/<chunk_id_sanitized>.bundle.md` — **LLM input**; deterministic Markdown projection used by the LLM semantic review pass for intent alignment. Never pass this to `--bundle`. 2. **Stale-input check.** Compare `bundle.chunk_text_sha` with a re-hash of the on-disk chunk text in the bundle. Mismatch → exit `5`, re-run the bundle prep step. 3. **Run the checker.** Invoke [scripts/check-section-doc.py](./scripts/check-section-doc.py): ```powershell python .github/skills/qa-reviewer/scripts/check-section-doc.py ` --bundle docs/_shared/DEMO100.CBL/_bundles/DEMO100__procedure-division__B200-CONTROLLO-PARAMETRO__B200-EX.bundle.json ` --language en ``` Flags (preferred → legacy): - `--bundle <path>` — **preferred.** Read `source_path`, `chunk`, `chunk_text`, `chunk_text_sha`, `profile`, and `output_paths.<language>` from the bundle. Markdown path = `bundle.output_paths.<language>`. - `--language <en|it|…>` — language key into `bundle.output_paths`; default `en`. - `--section-doc <path>` — override the bundle's resolved markdown path (rarely needed; skips the staleness check). - `--source <path> --chunk-id <id> --language <en|it|…>` — *legacy* derivation path retained for ad-hoc runs without a bundle; does NOT perform the `chunk_text_sha` staleness check. - `--profile <path>` — override profile.yaml (default `config/templates/section-doc.profile.yaml`). - `--json-out <path>` — override review sidecar path (default `docs/_reviews/section-doc/<file>__<chunk_id>__<lang>.json`). - `--no-write` — print review JSON to stdout, do not persist the sidecar. 4. **Report.** Print one summary line: `OK …` or `FAIL: N error(s), M warn(s) → <sidecar path>`. On failure, print one line per `error` finding (`[CODE] line=… <message>`). ## Determinism Rules - Mechanical checks are 100% deterministic: byte-identical inputs → byte-identical sidecar. - The sidecar JSON sorts `findings[]` by `(severity desc, line asc, code asc)` and uses stable key order so re-runs are diff-clean. - The semantic LLM review run is **separate** from this skill and produces a sibling sidecar (or appends to the same JSON under a distinct `semantic_findings[]` key). Do not mix the two in the same write — the LLM half is non-deterministic and should be re-runnable independently. - Profile thresholds are read once at the top of the run and copied verbatim into `thresholds` so the sidecar is self-describing without re-loading the profile. ## Whole-source section sweep After section authoring, run [scripts/review-section-set.py](./scripts/review-section-set.py) to validate the complete indexed section set: ```powershell python .github/skills/qa-reviewer/scripts/review-section-set.py ` --source repos/sample/COBOL/ATCCDLGH.CBL ``` - Languages come from `output.languages` in [pipeline.yaml](../../../config/pipeline.yaml); `--languages it,en` overrides them. English is the fallback only when the YAML language setting is omitted. Codes are normalized and deduplicated using the public runners' resolver; empty or malformed lists are rejected. - Before invoking any checker, the sweep requires a current source manifest, a non-empty, unambiguous chunks index, every indexed markdown for every requested language, and its authoritative `_bundles/<chunk-filename-stem>.bundle.json`. Missing directories/documents/bundles and orphan markdown files block the sweep. Unrequested language trees are not checked or removed. - Bundle source, chunk ID, and language output paths must match the indexed documents, and `facts_slice` must be present. Each checker receives `--bundle`, so incoming-reference rules and embedded-text hash checks are not bypassed. Re-stage bundles when adding languages or when inputs change. - `--source-root` and `--profile` remain available. Current child stdout/stderr and failure exit codes are printed. Existing sidecars are never read to infer the current run's findings; normal completed checks still write their sidecars. - Exit codes: `0` complete success (warnings allowed); `1` content/checker failure; `2` invalid CLI/configuration; `4` missing or invalid prerequisites, incomplete sections, or orphan documents; `5` stale source/bundle. Mixed child failures prioritize `4`, then `5`, then `1`. Preflight failures stop before any reviews. ## Resources - [scripts/check-section-doc.py](./scripts/check-section-doc.py) — deterministic mechanical checker (entry point). - [scripts/review-section-set.py](./scripts/review-section-set.py) — complete indexed section-set gate. ## Pipeline Wiring