Skip to content

fix: 8 code-review findings — front-matter guard, gate cwd fallback, dedup (v5.68.1)#50

Merged
edwinhu merged 1 commit into
mainfrom
fix-code-review-8-findings
Jul 5, 2026
Merged

fix: 8 code-review findings — front-matter guard, gate cwd fallback, dedup (v5.68.1)#50
edwinhu merged 1 commit into
mainfrom
fix-code-review-8-findings

Conversation

@edwinhu

@edwinhu edwinhu commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes 8 verified code-review findings (3 correctness, 5 cleanup) surfaced against skills/docx-repair/scripts/fix_footnotes.py, hooks/mechanical-floor-gate.py / hooks/writing-mechanical-gate.py, and skills/law-review-docx/scripts/build_docx.py.

  1. CONFIRMED bug_count_leading_tab_indents/fix_leading_tab_indents were missing the seen_h1 front-matter guard their siblings (normalize_body_indent, _count_unstyled_body_indents, apply_template_body_styles) all enforce. Added the guard to both (via a shared _iter_leading_tab_paras generator — see Your Strunk & White linter as a live editor -- paste a paragraph, get it back tighter #6). Updated skills/docx-repair/SKILL.md (~line 364) to document the front-matter guard applies here too.
  2. CONFIRMED bug_project_from_args only read args.projectDir, which Agent tool calls never carry, so FLOOR=dev always audited "." (the hook process's own cwd) instead of the real project. Fixed by falling back to hook_input["cwd"] (threaded through from main()) before defaulting to ".". Applied identically to the ds branch and to writing-mechanical-gate.py's call site, via the shared helper in cite-check: materialize-sources writes stale .md file paths to sources.bib #4.
  3. CONFIRMED edge bugfix_leading_tab_indents silently returned [] when tab-led paragraphs existed but no paragraph anywhere had a real firstLine indent to infer the dominant value from — contradicting detect_issues (which still reports leading_tab_indent(N)) and the SKILL.md "Logged, never silent" claim. Now returns a WARNING: N leading-tab paragraph(s) detected but no real firstLine indent exists to infer the dominant value — not fixed change entry instead.
  4. Reuse — Extracted hooks/_gate_common.py (deny, _project_from_args) shared by both mechanical-floor-gate.py and writing-mechanical-gate.py, which duplicated them byte-for-byte. Each hook still runs standalone via uv run python3 <path> — imports the sibling module with a sys.path.insert(0, str(HOOKS_DIR)) since hooks run from an unknown cwd. _run_dev/_run_ds/_run_check_all (dev/ds/writing-specific check-all invocations) were left in their respective hook files — the existing regression tests (tests/test_mechanical_floor_gate.py, tests/test_writing_mechanical_gate.py) assert on literal source-text of each hook's own file (e.g. '"--with", "lxml"' in src), so extracting those would have broken the tests without adding real dedup value (they already differ per-branch: CHECK_ALL_PY vs CHECK_ALL_DS vs CHECK_ALL).
  5. Reuse — Extracted _dominant_firstline(paras) (the dominant-firstLine Counter loop) and _ensure_ind(pPr) (the schema-ordered <w:ind> create-and-insert block), previously duplicated between normalize_body_indent and fix_leading_tab_indents. Both functions now call the shared helpers; insertion-point behavior (before rPr/sectPr, else appended) is preserved exactly.
  6. Simplification — Extracted _iter_unstyled_body_paras(root) and _iter_leading_tab_paras(root) generators, each shared by its count function and its fix function (mirroring _iter_bio_ref_runs). The front-matter guard fix from Add ETL pipeline incremental testing pattern to ds workflow #1 lives in these shared iterators, so both the detector and the fixer get it for free.
  7. Efficiency (--restyle-body 3x round-trip)Skipped. Given findings 5/6 already land a refactor in this file, layering the round-trip-collapse on top risked destabilizing three interacting passes for a perf win that wasn't asked to be prioritized. Left as future work.
  8. Consistencybuild_docx.py's convert_to_pdf now prints a loud WARNING: Word unavailable — PDF rendered via x2t/LibreOffice, layout may reflow; re-render with --renderer word before submission to stderr when it takes the non-Word fallback path, instead of silently contradicting the docx-render Iron Law ("--renderer word for deliverables"). doc_render.py itself was not touched.

Test plan

  • uv run python3 tests/test_mechanical_floor_gate.py — 11/11 passed
  • uv run python3 tests/test_writing_mechanical_gate.py — 8/8 passed
  • python3 -m py_compile on every edited .py file (fix_footnotes.py, both hooks, _gate_common.py, build_docx.py)
  • FLOOR=dev uv run python3 hooks/mechanical-floor-gate.py . and uv run python3 hooks/writing-mechanical-gate.py . — both run cleanly (no crash) in CLI debug mode
  • Synthetic-XML smoke tests against fix_footnotes.py (no fixture docx exists in-repo): front-matter guard preserves a leading-tab paragraph before Heading1 while fixing one after it; the "no real firstLine anywhere" case returns the WARNING and leaves the doc byte-identical; fix_leading_tab_indents is idempotent (second run produces no changes); _count_unstyled_body_indents/apply_template_body_styles count/apply parity holds after the shared-iterator refactor

Version

Bumped 5.68.0 → 5.68.1 in all 3 locations (.claude-plugin/plugin.json, .claude-plugin/marketplace.json metadata.version + plugins[0].version).

🤖 Generated with Claude Code

https://claude.ai/code/session_01J3NEAUUosUHyci4iFZBH2J

…ack (v5.68.1)

Fixes 8 verified code-review findings:

1. fix_footnotes.py leading-tab indent functions now share the same
   seen_h1 front-matter guard as their siblings (normalize_body_indent,
   apply_template_body_styles) so title/abstract/TOC paragraphs are never
   touched.
2. mechanical-floor-gate.py / writing-mechanical-gate.py now fall back to
   hook_input["cwd"] when args.projectDir is absent (Agent tool calls never
   carry it), fixing FLOOR=dev auditing "." instead of the real project.
3. fix_leading_tab_indents now emits a WARNING change entry instead of
   silently no-oping when tab-led paragraphs exist but no real firstLine
   indent exists to infer the dominant value from.
4. Extracted hooks/_gate_common.py (deny + _project_from_args) shared by
   both gate hooks; each hook still runs standalone via sys.path-insert.
5. Extracted _dominant_firstline() and _ensure_ind() helpers, used by both
   normalize_body_indent and fix_leading_tab_indents.
6. Extracted _iter_unstyled_body_paras()/_iter_leading_tab_paras()
   generators shared by each pass's count and fix functions.
7. Skipped the 3-round-trip main() perf refactor (--restyle-body path) —
   not low-risk enough given the findings-5/6 refactor already landing in
   the same file.
8. build_docx.py's convert_to_pdf now prints a loud stderr WARNING when it
   falls back to the non-Word x2t/LibreOffice path, instead of silently
   contradicting the docx-render "--renderer word for deliverables" law.

Tested: tests/test_mechanical_floor_gate.py (11/11) and
tests/test_writing_mechanical_gate.py (8/8) pass; both hooks' CLI debug
modes run cleanly; py_compile on all edited files; synthetic-XML smoke
tests for the front-matter guard, WARNING path, and count/fix parity in
fix_footnotes.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J3NEAUUosUHyci4iFZBH2J
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant