kaykayyali/damascus-orchestrator
1
0
Fork 0
Code Issues 3 Pull Requests Actions Packages Projects Releases Wiki Activity

test(contract): spec-refiner must route ANY non-empty Ambiguities to awaiting_human #11

Closed
kaykayyali wants to merge 0 commits from test/spec-refiner-ambiguity-routing into main
pull from: test/spec-refiner-ambiguity-routing
merge into: kaykayyali:main
Conversation 2 Commits - Files Changed - +56
kaykayyali commented 2026-06-24 07:11:20 +00:00
Owner
Copy Link

Contract: wiki/concepts/spec-refiner-contract.md AC line ~122 — any non-empty ## Ambiguities section routes the row to phase=awaiting_human and opens a human_issue.

Bug: src/damascus/phases.py:74 guards on re.search(r"\?\s*$", _section(text, "Ambiguities")) — the section must end in ? to trigger. A section that lists an ambiguity without a trailing question mark falls through to phase=build, the human never sees the issue.

Gap note: wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md (with Option A: drop the ? check, replace with non-empty section check; 5-min fix).

Companion contract test: PR #10 (test/spec-refiner-prompts-row-constraints) covers the file_scope/budget_cycles aspect of the same spec-refiner contract. This PR covers the ambiguity-routing aspect.

Companion E2E test: the contract test passes when the fix lands (structural); an E2E test in tests/e2e/ will pass when the runtime behavior is right (behavioral). The E2E is out of scope for this PR — would need a synthetic ambiguous spec + a live cycle run.

Source-grep gotcha: the bug pattern uses re.escape(r"\?\s*$") so the literal 6 chars \ ? \ s * $ in source match character-for-character. Without re.escape(), raw-string regex interprets \? as literal ? (no backslash), \s as whitespace class (no \s text), \$ as literal $ (no backslash) — and the test passes on broken code (false negative).

Status: RED on main — the test fails today, goes green on the Option A fix. Marked RED-in-main per the judgment-call in references/contract-test-pattern.md: the visible-in-CI failure is the to-do item for the next PR-author.

Scope: one file, +56 lines, one test function. No source change. Reviewable in 2 min.

**Contract:** wiki/concepts/spec-refiner-contract.md AC line ~122 — any non-empty `## Ambiguities` section routes the row to `phase=awaiting_human` and opens a `human_issue`. **Bug:** `src/damascus/phases.py:74` guards on `re.search(r"\?\s*$", _section(text, "Ambiguities"))` — the section must end in `?` to trigger. A section that lists an ambiguity without a trailing question mark falls through to `phase=build`, the human never sees the issue. **Gap note:** wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md (with Option A: drop the `?` check, replace with non-empty section check; 5-min fix). **Companion contract test:** PR #10 (`test/spec-refiner-prompts-row-constraints`) covers the file_scope/budget_cycles aspect of the same spec-refiner contract. This PR covers the ambiguity-routing aspect. **Companion E2E test:** the contract test passes when the fix lands (structural); an E2E test in `tests/e2e/` will pass when the runtime behavior is right (behavioral). The E2E is out of scope for this PR — would need a synthetic ambiguous spec + a live cycle run. **Source-grep gotcha:** the bug pattern uses `re.escape(r"\?\s*$")` so the literal 6 chars `\` `?` `\` `s` `*` `$` in source match character-for-character. Without `re.escape()`, raw-string regex interprets `\?` as literal `?` (no backslash), `\s` as whitespace class (no `\s` text), `\$` as literal `$` (no backslash) — and the test passes on broken code (false negative). **Status:** RED on `main` — the test fails today, goes green on the Option A fix. Marked RED-in-main per the judgment-call in `references/contract-test-pattern.md`: the visible-in-CI failure is the to-do item for the next PR-author. **Scope:** one file, +56 lines, one test function. No source change. Reviewable in 2 min.
kaykayyali added 1 commit 2026-06-24 07:11:21 +00:00
test(contract): spec-refiner must route ANY non-empty Ambiguities to awaiting_human
Some checks failed
test / contract-and-unit (pull_request) Failing after 12s
Details
bb96bdb83f 
The contract at wiki/concepts/spec-refiner-contract.md says a non-empty
`## Ambiguities` section routes the row to phase=awaiting_human and
opens a human_issue. The current implementation in phases.py:74 guards
on re.search(r"\?\s*$", _section(text, "Ambiguities")) — this
requires the section to end with `?`, so a section that lists an
ambiguity without a trailing question mark falls through to build.

This is the structural counterpart to the E2E test that runs the
spec-refiner on a synthetic ambiguous spec and asserts the row lands
in awaiting_human regardless of trailing punctuation. Both tests are
RED today; both go green on the same fix (the documented Option A in
wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md:
drop the `?` check, replace with a non-empty check on the section).

Uses re.escape() on the bug pattern so the literal text `\?\s*$`
in source matches character-for-character (raw-string regex would
interpret \? as literal ? and \s as whitespace class, false-passing
on broken code per the skill's Source-grep gotcha).

Co-Authored-By: Claude <noreply@anthropic.com>
kaykayyali commented 2026-06-24 07:11:49 +00:00
Author
Owner
Copy Link

Self-review (heartbeat, 2026-06-24 07:08)

Diff: one file, +56 lines, one test function in tests/contract/test_contracts_match_source.py. Branch test/spec-refiner-ambiguity-routing off main, no source change.

Why this PR exists:

The spec-refiner contract (wiki/concepts/spec-refiner-contract.md AC line ~122) says any non-empty ## Ambiguities section routes the row to phase='awaiting_human' and opens a human_issue. The current phases.py:74 implementation requires the section to end in ?. These don't match. A spec whose Ambiguities section lists a question without trailing ? falls through to phase='build', the human never sees the issue, and the build phase either guesses or crashes.

RED-on-broken-code verification:

bug_pattern = re.escape(r"\?\s*$")  # literal 6 chars in source
# matches broken refine_body: True
# matches Option-A-fixed refine_body: False
# awaiting_human in fixed: True
# open_human_issue in fixed: True

Test correctly goes RED today, GREEN on the Option A fix from the gap note.

Source-grep gotcha (2026-06-24 skill pitfall): the bug pattern uses re.escape() so the literal \?\s*$ in source matches character-for-character. Raw-string regex interprets \? as literal ?, \s as whitespace class, \$ as literal $ — and would false-pass on broken code. Verified end-to-end on the related PR #10 (file_scope/budget_cycles contract) before applying the same pattern here.

Companion work:

  • PR #10 (test/spec-refiner-prompts-row-constraints) covers the file_scope/budget_cycles aspect of the same contract. Both PRs together cover both contract-drift bugs identified in the spec-refiner contract.
  • Gap note: wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md — has the 3 options + recommendation (Option A: drop the ? check, ~5 min).
  • E2E counterpart not in this PR — would need a synthetic ambiguous spec + live cycle run. Out of scope.

What's NOT in this PR:

  • The fix itself (Option A from the gap note). Per gap-finding discipline, code-level decisions need an LLM session, not a heartbeat tick. This PR ships the test that catches the bug; the fix is a separate PR.
  • E2E test. Structural test catches the literal; E2E would catch behavioral drift (e.g. the section gets parsed differently in _section()). Pair-up recommendation in the gap note.
  • Amendment §7 / §2 / §3 work — still gated on issue #6 BMAD picks.

Merge recommendation: ship — 1 file, +56 lines, test is RED on broken code as verified, fix is documented in the gap note. The fix PR (when written) takes 5 min and goes green on this same test.

## Self-review (heartbeat, 2026-06-24 07:08) **Diff:** one file, +56 lines, one test function in `tests/contract/test_contracts_match_source.py`. Branch `test/spec-refiner-ambiguity-routing` off main, no source change. **Why this PR exists:** The spec-refiner contract (`wiki/concepts/spec-refiner-contract.md` AC line ~122) says **any non-empty `## Ambiguities` section** routes the row to `phase='awaiting_human'` and opens a `human_issue`. The current `phases.py:74` implementation requires the section to end in `?`. These don't match. A spec whose Ambiguities section lists a question without trailing `?` falls through to `phase='build'`, the human never sees the issue, and the build phase either guesses or crashes. **RED-on-broken-code verification:** ```python bug_pattern = re.escape(r"\?\s*$") # literal 6 chars in source # matches broken refine_body: True # matches Option-A-fixed refine_body: False # awaiting_human in fixed: True # open_human_issue in fixed: True ``` Test correctly goes RED today, GREEN on the Option A fix from the gap note. **Source-grep gotcha (2026-06-24 skill pitfall):** the bug pattern uses `re.escape()` so the literal `\?\s*$` in source matches character-for-character. Raw-string regex interprets `\?` as literal `?`, `\s` as whitespace class, `\$` as literal `$` — and would false-pass on broken code. Verified end-to-end on the related PR #10 (file_scope/budget_cycles contract) before applying the same pattern here. **Companion work:** - PR #10 (`test/spec-refiner-prompts-row-constraints`) covers the file_scope/budget_cycles aspect of the same contract. Both PRs together cover both contract-drift bugs identified in the spec-refiner contract. - Gap note: `wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md` — has the 3 options + recommendation (Option A: drop the `?` check, ~5 min). - E2E counterpart not in this PR — would need a synthetic ambiguous spec + live cycle run. Out of scope. **What's NOT in this PR:** - The fix itself (Option A from the gap note). Per gap-finding discipline, code-level decisions need an LLM session, not a heartbeat tick. This PR ships the test that catches the bug; the fix is a separate PR. - E2E test. Structural test catches the literal; E2E would catch behavioral drift (e.g. the section gets parsed differently in `_section()`). Pair-up recommendation in the gap note. - Amendment §7 / §2 / §3 work — still gated on issue #6 BMAD picks. **Merge recommendation:** ship — 1 file, +56 lines, test is RED on broken code as verified, fix is documented in the gap note. The fix PR (when written) takes 5 min and goes green on this same test.
kaykayyali commented 2026-06-24 09:03:26 +00:00
Author
Owner
Copy Link

Closing this PR — superseded by PR #12.

PR #12 (fix(spec): route any non-empty Ambiguities section to awaiting_human) was branched off the tip of PR #11 (bb96bdb) and includes the same test from this PR plus the 1-line source fix in phases.py:74. Verified: git diff bb96bdb..abc8f4a -- tests/contract/test_contracts_match_source.py returns 0 lines — byte-identical test file.

Recommended merge order: PR #12 first (carries test + fix as one commit on top of main). PR #11 would be a no-op duplicate after that.

The contract test that lives in both PRs is the structural counterpart to the spec-refiner-contract.md §3 ("any non-empty ## Ambiguities section triggers awaiting_human"). The 1-line fix removes the ?-terminator check that was silently swallowing list-style ambiguities and routing them to build. Verified RED on broken (with the bug pattern literal re-introduced) and GREEN on the fix. See gap note wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md for the full trace.

Closing this so the inbox stays clean. PR #12 has the full test+fix as a single PR; merging it carries both.

**Closing this PR — superseded by PR #12.** PR #12 (`fix(spec): route any non-empty Ambiguities section to awaiting_human`) was branched off the tip of PR #11 (`bb96bdb`) and includes the same test from this PR plus the 1-line source fix in `phases.py:74`. Verified: `git diff bb96bdb..abc8f4a -- tests/contract/test_contracts_match_source.py` returns 0 lines — byte-identical test file. **Recommended merge order:** PR #12 first (carries test + fix as one commit on top of main). PR #11 would be a no-op duplicate after that. The contract test that lives in both PRs is the structural counterpart to the spec-refiner-contract.md §3 ("any non-empty `## Ambiguities` section triggers awaiting_human"). The 1-line fix removes the `?`-terminator check that was silently swallowing list-style ambiguities and routing them to `build`. Verified RED on broken (with the bug pattern literal re-introduced) and GREEN on the fix. See gap note `wiki/queries/damascus-orchestrator/spec-refiner-ambiguity-routing-drift-2026-06-24.md` for the full trace. Closing this so the inbox stays clean. PR #12 has the full test+fix as a single PR; merging it carries both.
kaykayyali closed this pull request 2026-06-24 09:03:30 +00:00
Some checks failed
test / contract-and-unit (pull_request) Failing after 12s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kaykayyali
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kaykayyali/damascus-orchestrator#11
Delete Branch "test/spec-refiner-ambiguity-routing"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?