From 8a4ca754054b1f529a0c1b160f2b7a007bf75f69 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:37:17 +0000 Subject: [PATCH] Fix converge-coverage-denominators: add per-category denominators, key ledger, and incomplete_assessment outcome Apply the remediation from the bug assessment on issue #3752. - Replace undenominated Step 6 summary with per-category checked/total metrics - Add coverage ledger table (one row per inventory key with status) - Add incomplete_assessment fail-closed outcome in Step 7 - Update Step 8 handoff and Step 9 outcome list - Add regression test pinning the contract Refs #3752 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- templates/commands/converge.md | 43 +++++++++++--- tests/test_converge_template_contract.py | 74 ++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 tests/test_converge_template_contract.py diff --git a/templates/commands/converge.md b/templates/commands/converge.md index eadb96ee58..ca268d330c 100644 --- a/templates/commands/converge.md +++ b/templates/commands/converge.md @@ -185,17 +185,39 @@ Before appending anything, output a compact, severity-graded summary (no file wr |----|----------|----------|--------|----------|----------------| | F1 | missing | HIGH | FR-008 | Example: no append-only guard detected in path/to/module.py when writing tasks.md | Add append-only enforcement | -**Summary metrics:** +**Summary metrics** (all denominators derived from the inventory built in Step 3, not from model memory): -- Requirements / acceptance criteria checked -- Plan decisions checked -- Constitution principles checked (or "skipped — template") +- Functional Requirements (FR-###): `/` checked +- Success Criteria (SC-###): `/` checked +- User-story acceptance scenarios (US#/AC#): `/` checked +- Plan decisions: `/` checked +- Constitution principles: `/` checked (or "skipped — template") - Findings by gap type (missing / partial / contradicts / unrequested) - Findings by severity -### 7. Append Convergence Tasks (or report converged) +**Coverage ledger** (one row per inventory key — include every key from Step 3): -**If there are one or more actionable findings** (`tasks_appended` outcome): +| Key | Status | Finding | +|-----|--------|---------| +| FR-001 | ✅ satisfied | — | +| SC-002 | ⚠️ gap | → F1 | +| US1/AC2 | — unassessed | — | + +Status values: `✅ satisfied`, `⚠️ gap → Finding Fxx`, `— unassessed`. + +> **Fail-closed rule**: if any inventory key carries `— unassessed` status, the command +> MUST use the `incomplete_assessment` outcome (Step 7) and MUST NOT emit `converged` or +> append tasks. + +### 7. Append Convergence Tasks (or report incomplete / converged) + +**If any inventory key is unassessed** (`incomplete_assessment` outcome): + +- Do **not** modify `tasks.md` at all. +- Report: **"⚠️ Incomplete assessment — N inventory key(s) were not assessed. Re-run `/speckit.converge` after addressing the coverage gaps, or verify that the spec artifacts contain all required identifiers."** +- List the unassessed keys by category so the operator knows exactly what was skipped. + +**If there are one or more actionable findings and all inventory keys are assessed** (`tasks_appended` outcome): Append to the **end** of `tasks.md`, per the append contract: @@ -219,14 +241,17 @@ Append to the **end** of `tasks.md`, per the append contract: 4. Never reuse or renumber existing IDs. If a prior Convergence phase exists, add a new, separately-numbered one below it — do not touch the old one. -**If there are no actionable findings** (`converged` outcome): +**If there are no actionable findings and all inventory keys are assessed** (`converged` outcome): - Do **not** modify `tasks.md` at all — no empty phase header. - Report: **"✅ Converged — the implementation satisfies the spec, plan, and tasks."** -- Include the summary counts of what was checked. +- Include the per-category `checked/total` summary counts. ### 8. Provide Next Actions (Handoff) +- On `incomplete_assessment`: explain that N key(s) were not assessed (list them), and + instruct the operator to re-run `/speckit.converge` after ensuring the agent reads all + relevant source files or after resolving any ambiguities in the spec artifacts. - On `tasks_appended`: state how many tasks were appended under which phase, and recommend running `__SPECKIT_COMMAND_IMPLEMENT__` to complete them; note that a follow-up converge run will find fewer or no remaining items. @@ -243,7 +268,7 @@ After producing the result, check if `.specify/extensions.yml` exists in the pro - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation -- Report the convergence outcome (`converged` or `tasks_appended`) in-session before listing +- Report the convergence outcome (`converged`, `tasks_appended`, or `incomplete_assessment`) in-session before listing any hooks, so users can decide whether to run optional follow-up commands. - For each executable hook, output the following based on its `optional` flag: - **Optional hook** (`optional: true`): diff --git a/tests/test_converge_template_contract.py b/tests/test_converge_template_contract.py new file mode 100644 index 0000000000..999dc1821b --- /dev/null +++ b/tests/test_converge_template_contract.py @@ -0,0 +1,74 @@ +"""Regression test for the converge.md template output contract. + +Pins the three requirements introduced in issue #3752: +1. Step 6 summary metrics use per-category denominator syntax (checked/total). +2. Step 6 includes a coverage ledger table with key-level status. +3. Step 7 defines an ``incomplete_assessment`` fail-closed outcome. +""" + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).parent.parent +CONVERGE_TEMPLATE = REPO_ROOT / "templates" / "commands" / "converge.md" + + +def _text() -> str: + return CONVERGE_TEMPLATE.read_text(encoding="utf-8") + + +def test_per_category_denominator_syntax_present(): + """Step 6 must expose checked/total per category, not a bare aggregate.""" + text = _text() + # The template must include the denominator pattern for each major category. + assert "checked>/" in text or re.search(r"checked>/", text), ( + "converge.md Step 6 must use '/' denominator syntax " + "for per-category metrics." + ) + + +def test_coverage_ledger_table_present(): + """Step 6 must include a key-ledger table so reports are auditable.""" + text = _text() + assert "Coverage ledger" in text, ( + "converge.md must contain a 'Coverage ledger' section in Step 6." + ) + # The ledger table must define the three status values. + assert "✅ satisfied" in text + assert "⚠️ gap" in text + assert "unassessed" in text + + +def test_incomplete_assessment_outcome_present(): + """Step 7 must define an ``incomplete_assessment`` fail-closed outcome.""" + text = _text() + assert "incomplete_assessment" in text, ( + "converge.md Step 7 must define an 'incomplete_assessment' outcome " + "to fail closed when inventory keys are unassessed." + ) + + +def test_incomplete_assessment_blocks_tasks_md_write(): + """The incomplete_assessment branch must NOT write tasks.md.""" + text = _text() + # Locate the incomplete_assessment section and verify the prohibition. + idx = text.find("incomplete_assessment` outcome") + assert idx != -1 + # Within the next 500 chars after the outcome heading, the template must + # say "Do not modify" tasks.md. + excerpt = text[idx : idx + 500] + assert "Do **not** modify" in excerpt or "do not modify" in excerpt.lower(), ( + "The incomplete_assessment branch must instruct the agent NOT to modify tasks.md." + ) + + +def test_step_8_handoff_covers_incomplete_assessment(): + """Step 8 must provide a next-action note for incomplete_assessment.""" + text = _text() + # Step 8 handoff section must reference incomplete_assessment explicitly. + step8_idx = text.find("### 8.") + assert step8_idx != -1 + step8_text = text[step8_idx : step8_idx + 600] + assert "incomplete_assessment" in step8_text, ( + "Step 8 handoff must include guidance for the incomplete_assessment outcome." + )