SSA full drift checker (template + staging headers)#16
Conversation
Lifts the 4 inline placeholder strings ({{SITE_ADDRESS}},
{{NARRATIVE_SUMMARY}}, {{AUDIT_DATE}}, {{PREPARED_BY}}) out of
build_ssa_report_docx into module-level PH_* constants plus a
SSA_REPORT_PLACEHOLDERS tuple. Pure refactor — no behaviour change,
114 tests unchanged.
Prerequisite for the full drift checker, which needs an enumerable
constant surface to walk against the template docx.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two checks on top of the minimal v1 CSV header check:
- SSA report template carries every PH_* placeholder. Walks the
template docx's word/*.xml parts, concatenates <w:t> text content
(handles placeholders split across formatting boundaries), and
asserts each token in SSA_REPORT_PLACEHOLDERS appears somewhere.
- Every STAGING_HEADERS entry has a _STAGING_COL_WIDTHS entry.
Subset semantics — _STAGING_COL_WIDTHS legitimately carries
forward-defined gap-4/5/6 columns (phase, hrcw, swms_*, etc.)
not yet in STAGING_HEADERS.
3 checks total now, all green on current main. 5 tests cover both
the clean path and per-check failure paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d04355855b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| haystack = "".join(parts) | ||
| missing = [ph for ph in SSA_REPORT_PLACEHOLDERS if ph not in haystack] |
There was a problem hiding this comment.
Reject split placeholders that the builder cannot replace
When Word splits a placeholder across formatting runs, this join makes the drift check pass because {{SITE_ + ADDRESS}} is found in the concatenated haystack, but build_ssa_report_docx still replaces only tokens that appear wholly inside one <w:t> (_replace_tokens_in_part iterates individual text nodes). In that edited-template scenario the checker reports green while the generated report keeps unreplaced placeholder pieces, so the check should either require each placeholder within a single text node or the builder needs matching split-run replacement support.
Useful? React with 👍 / 👎.
Summary
Follow-up to #15. Promotes the minimal drift checker (1 check) to a full drift checker (3 checks) by first centralising the SSA report placeholder tokens.
build_ssa_report_docx's inline{{SITE_ADDRESS}}/{{NARRATIVE_SUMMARY}}/{{AUDIT_DATE}}/{{PREPARED_BY}}literals are now module-levelPH_*constants plus anSSA_REPORT_PLACEHOLDERStuple. Pure refactor — 114 docx-build tests unchanged..docx'sword/*.xmlparts, concatenates<w:t>text content (so placeholders split across formatting boundaries are still caught), and asserts eachPH_*appears.STAGING_HEADERSentry must have a corresponding_STAGING_COL_WIDTHSkey. Subset semantics —_STAGING_COL_WIDTHSlegitimately carries forward-defined gap-4/5/6 columns (phase, hrcw, swms_required, etc.) not yet inSTAGING_HEADERS.3 checks total, all green on current branch. 5 drift-checker tests cover the clean path and per-check failure paths.
Test plan
python tools/check_ssa_drift.py— exits 0 with 3 [X] markspython -m pytest tests/test_ssa_drift_checker.py— 5 tests passpython -m pytest tests/test_ssa_pipeline.py tests/test_ssa_determinism.py tests/test_ssa_oxml_validator.py— docx-build behaviour unchanged after the placeholder refactor (114 passing)Notes for reviewer
<w:t>...</w:t>regex to extract Word's visible text content rather than substring-searching the raw XML, because Word may split a placeholder across multiple text runs when formatting boundaries land mid-token._STAGING_COL_WIDTHShas 6+ forward-defined gap-4/5/6 entries not yet inSTAGING_HEADERS. Asymmetric semantics catch the case that matters (missing widths) without requiring those forward-defined entries to be removed.🤖 Generated with Claude Code