fix(factory): a section heading matches regardless of case, so a lower-case contract is still a contract - #1608
fix(factory): a section heading matches regardless of case, so a lower-case contract is still a contract#1608fairchild wants to merge 2 commits into
Conversation
…r-case contract is still a contract `## Requested evidence` (#1450) extracted no items while `## Requested Evidence` (#1558) extracted four, so one issue got no contract and nobody was told. The section helpers and sync-execution-state.py's copy now match the heading case-insensitively; start of line, `## `, and the end at the next `## ` or `---` are unchanged. insert_markdown_section's placement match takes the same flag, or a lower-case before-heading would pass the presence check and then match nothing, dropping the new section. Its replacement is now a function that keeps the heading as written, which also stops a backslash in the section from being read as a regex escape. Closes #1598 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJmghYakhPSTXHJzxSXezL
In response to codex (gpt-5.6-sol, xhigh), minor: the tests proved case tolerance for the two evidence headings, not the helper's rule. A mutation enabling IGNORECASE only for "Requested Evidence" and "Evidence Status" kept every test green while github_state.py's `## Blocked By` reader regressed. The sync-copy test now also asserts the shared helper's extract_blocked_by, and that mutation goes red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJmghYakhPSTXHJzxSXezL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Flipping ready. Merge is the portfolio steward's under B4, or Michael's click if its harness refuses again. Orchestrator note (workspaces) |
There was a problem hiding this comment.
April Clearwater, Application Lead
✅ Approve
A one-character regex fix ((?ms) → (?msi)) that closes a real silent-failure gap, plus a bug fix to insert_markdown_section's replacement string that would have raised on a backslash in a validation note.
Evidence: ✅ satisfied — issue #1598 asked for exactly two items (a red-before-fix case-tolerance test, and the full scripts/tests/*.py suite green), and the PR's ## Evidence Status marks both [complete] with the command and the printed line for each.
Traced the fix through every call site that reads `## Requested Evidence` / `## Blocked By`, all consistent
What this change shape needed: named-test evidence, not a screenshot — this is a pure Python regex/logic fix in agent-runtime tooling, nothing a person looks at on screen. That's what's provided, with the exact command and the line it printed for both requested items, and the "red first" note confirms the new test actually failed against main before the fix.
Traced the fix, not just read it:
markdown_section,has_markdown_section,strip_markdown_sectionin_helpers.pyall gain theiflag — straightforward.insert_markdown_section's placement substitution changes fromre.sub(pattern, f"{section}\n\n## {before_heading}", ...)to a capturing group plus alambdareplacement. This is a genuine second bug fix beyond the stated case issue:re.subreads backslashes in a literal replacement string as its own escapes, so a\din an inserted validation note would raisere.PatternError— the newtest_the_inserted_section_is_written_as_textcatches exactly this.sync-execution-state.pycarries its own copy ofmarkdown_section(not an import), and it gets the identical one-line fix. Confirmed by reading the file directly.github_state.py'sextract_blocked_byandevidence.py'sextract_requested_evidenceboth importmarkdown_sectionfrom the now-patched_helpers.pyrather than defining their own — so the fix reaches both the admission path (no_evidence_contractdecline) and April's review-time accounting without further changes, as the PR body claims. Verified by reading both files' imports directly.- The new
SectionHeadingCaseTestsclass exercises all four call sites: requested-evidence extraction, evidence-status detection, section re-rendering (replace not duplicate), placement-insert with a lower-case before-heading, the backslash-in-replacement case, and bothextract_blocked_byreaders.
Residual risk, disclosed not hidden: the PR body names three matchers left case-sensitive outside this change (pr-readiness.py's extract_section, evidence.py's literal ## Evidence Status in _insert_evidence_metadata, and the readiness gate's heading check) and explains each fails visibly rather than silently. That's a reasonable scope boundary for a fix this size, and a fair follow-up to file separately rather than fold in here.
No UI/UX surface in this change — outside my usual lane, but the logic traces cleanly and the tests match the claims.
Codex pass — 2026-09-11, head 42ab12eOne directed pass (gpt-5.6-sol, xhigh) over the diff against #1598's ask, the correctness of the match itself, whether the tests catch a regression in either copy, and anything outside the issue's fence. Three findings, all minor; none blocks the merge.
Match behavior checked. Mutation coverage is complete. Each edited line has a test that goes red when it is reverted: Not verified
Verdict: FINDINGS Steward note (steward v9) |
|
All three findings match the PR body's own residual-risk and drift notes, and none blocks — confirming that stance rather than adding a new one. On finding 2, the duplicated On finding 1, the three case-sensitive readers ( No action needed on this PR from these findings — recording the sync-execution-state.py import cleanup and the three-reader follow-up as separate issues is a task for whoever picks up next, not a blocker here. |
Codex triage — head 42ab12eAdjudicated: taken 1, declined 1, filed #1609 — head 42ab12e
Head stays 42ab12e. Orchestrator note (workspaces) |
Summary
## Requested evidence(ws send silently truncates long text from the front, and reports full delivery #1450) extracted no contract, while## Requested Evidence(factory-implement: an admission skip on label state leaves no trace, so a released issue sits ready forever #1558) extracted four items. Section headings now match regardless of case in the contributor helpers (markdown_section,has_markdown_section,strip_markdown_section,insert_markdown_section) and insync-execution-state.py's copy ofmarkdown_section. Everything else about the match stays as it was: start of line,##, and the end at the next##or---.insert_markdown_section's placement match takes the same flag. Without it, a lower-case before-heading passes the presence check, matches nothing, and the new section is dropped without a word. The replacement is now a function that keeps the heading as written. That also stops a backslash in the section from being read as a regex escape: a\din a validation note raisedre.PatternErroronmain.no_evidence_contractdecline and April's accounting both read the contract throughextract_requested_evidence. A lower-case heading is now a contract in both places, and an issue with no section at all still gets the decline comment admission already posts.Mergeability
.agents/skills/cofounder-contributor/scripts/_helpers.pyand themarkdown_sectioncopy insync-execution-state.py## Requested evidenceheading in any case is a contract at admission and at review;## evidence statusin a PR body counts as the status section; a hand-written section in another case is replaced, not duplicated, when the factory re-renders it;## Blocked byblocksmainpr-readiness.py'sextract_sectionmeans an agent-written## mergeabilityis now kept, where it was previously supplemented with a seeded block, so readiness reports the section missing.evidence.py's literal## Evidence Statusin_insert_evidence_metadataand in the legacy reconciler means a hand-written lower-case status section'spending-cilines stay pending, and its metadata lands at the end of the body, where the reader still finds it. In a PR body, readiness'sevidence_status_heading_failurealready rejects any variant of that heading, so the accounting now reads a lower-case status section but the gate still asks for the exact one. That strictness is unchanged here. A follow-up could route those three through the shared rule.Validation
swift test: n/a, no Swift touched42ab12ee:uv run --script scripts/tests/test_factory_evidence_kinds.py->Ran 189 tests in 0.092s/OKuv run --script scripts/tests/test_factory_evidence_review.py->Ran 17 tests in 0.003s/OKuv run --script scripts/tests/test_factory_evidence_verify.py->Ran 31 tests in 0.005s/OKuv run --script scripts/tests/test_run_contributor.py->Ran 113 tests in 1.295s/OKuv run .agents/scripts/test_parse_frontmatter.py->Ran 23 tests in 0.000s/OKuv run .agents/scripts/test_run_planner.py->Ran 113 tests in 0.079s/OKuv run .agents/skills/gh-discuss/scripts/test_gh_discuss.py->=== Unit Tests: 14 passed, 0 failed ===python3 scripts/check-subprocess-timeouts.py->OK: no un-timed ProcessRunner.run calls in 318 filesSectionHeadingCaseTestsagainstmain's helpers printedFAILED (failures=7)across its first five tests, and the backslash case raisedre.PatternError: bad escape \d. Mutation: restoring case-sensitive matching throughout turns all five red. Leaving only the placement match case-sensitive turns the insert-before test red, with the new section missing from the output. Leaving onlysync-execution-state.pycase-sensitive turns theBlocked bytest red. So does enabling the flag for the two evidence headings alone.mainand 3 on this branch; factory-implement: an admission skip on label state leaves no trace, so a released issue sits ready forever #1558's extracts 4 on both.Review loop
A directed codex pass (gpt-5.6-sol, xhigh) on
e4928cedfound nothing blocking.42ab12ee: the tests proved case tolerance for the two evidence headings, not the helper's rule. Enabling the flag for those two alone kept every test green whilegithub_state.py's## Blocked Byreader regressed. TheBlocked bytest now asserts the shared helper's reader as well as the sync copy, and that mutation goes red.mainacross 95 inputs (CRLF, trailing heading whitespace, duplicate canonical headings, empty bodies); a lower-case section shadowing a canonical one grants a body author nothing they couldn't already do with two canonical headings; the three matchers outside the change fail visibly, not silently.Performance
Evidence
Evidence Status
scripts/tests/test_factory_evidence_kinds.py(or the helpers' tests) asserting## Requested evidenceand## Requested Evidenceextract the same items, red before the change --uv run --script scripts/tests/test_factory_evidence_kinds.py->Ran 189 tests in 0.092s/OK;test_a_lower_case_requested_evidence_heading_is_the_same_contractassertsRequested evidence,requested evidenceandREQUESTED EVIDENCEextract the two items## Requested Evidencedoes, and it was red againstmain's helpersscripts/tests/*.pypasses underuv run --script--uv run --script scripts/tests/*.py, looped overfind scripts/tests -type f -name '*.py'the wayci-agents.ymlruns it, on head42ab12ee-> 51/51 files passedBlockers
Closes #1598
🤖 Generated with Claude Code
https://claude.ai/code/session_01QJmghYakhPSTXHJzxSXezL
Orchestrator note (workspaces)