From fa1ebd0707305ff060321d29514f1d97fb244523 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 19:07:46 +0000 Subject: [PATCH 01/12] Tighten [DOCS] story quality criteria and add re-evaluation on design-time changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Define seven numbered quality criteria for [DOCS] stories in decompose.md as a single authoritative source: searchable title, explicit reader/task, reader-verifiable AC, documentation scope, complete documentation inputs with fact/assumption/open-question distinction, contributing stories reference in Design Reference, and correct placement. Add re-evaluation gates so all design-time paths that create or modify [DOCS] stories check affected stories against these criteria: - /decompose: criteria are applied during story creation and verified by the decomposition review (criterion #8, updated to match). - /revise: new consistency check re-evaluates [DOCS] stories affected by direct edits or changes to their contributing DEV/UI stories. - /respond: new re-evaluation in Step 6 when review-driven design changes affect user-facing surfaces covered by [DOCS] stories. - /sync: new Step 3b validates [DOCS] stories before Jira creation/update, blocking low-quality tickets from reaching the documentation author. Update the DOCS story template to include a Contributing stories line in Design Reference for explicit traceability to implementation stories. Update test fixtures and add a test verifying Contributing stories reference resolution through sync rendering. Bump design workflow version 0.14.0 → 0.15.0 (MINOR — behavioral change). Co-Authored-By: Claude Opus 4.6 --- design/SKILL.md | 2 +- design/decomposition-review.md | 30 ++++--- design/scripts/test_sync_helper.py | 26 ++++++ design/skills/decompose.md | 138 +++++++++++++++++++---------- design/skills/respond.md | 13 +++ design/skills/revise.md | 27 ++++++ design/skills/sync.md | 24 +++++ 7 files changed, 200 insertions(+), 60 deletions(-) diff --git a/design/SKILL.md b/design/SKILL.md index 3171ea12..3c28fcb0 100644 --- a/design/SKILL.md +++ b/design/SKILL.md @@ -1,6 +1,6 @@ --- name: design -version: 0.14.0 +version: 0.15.0 description: >- Design-and-decompose workflow that takes a PRD, researches the problem space, drafts a technical design document with a requirement-anchored testplan, diff --git a/design/decomposition-review.md b/design/decomposition-review.md index f69db566..75484a82 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -144,15 +144,21 @@ impact: as gaps. Do not reject a legitimate prerequisite for adding no new feature behavior — that is its nature. -8. **Documentation** — Do `[DOCS]` stories use the `[DOCS]` template - (Documentation Scope and Documentation Inputs instead of - Implementation Guidance and Testing Approach)? Does the Documentation - Scope describe what the reader needs to understand without prescribing - how to write or organize the documentation? Does the Documentation - Inputs section inventory all user-facing artifacts from the - implementation stories it references? Is the inventory detailed enough - for a documentation author to know what to look for in the - implementation? +8. **Documentation** — Do `[DOCS]` stories satisfy all seven `[DOCS]` + story quality criteria from the decompose skill? Specifically: does + the title include searchable product/feature/command terms (not just + `[DOCS] Feature documentation`)? Does the persona/goal/benefit name + the actual reader and a concrete task? Are acceptance criteria + reader-verifiable documentation outcomes (not implementation + artifacts)? Does the Documentation Scope describe what the reader + needs to understand without prescribing how to write or organize the + documentation? Does the Documentation Inputs section inventory all + user-facing artifacts from the contributing `[DEV]`/`[UI]` stories, + with fact/assumption/open-question distinction? Does the Design + Reference include a `Contributing stories` line consistent with the + Documentation Inputs headers? Are the contributing story references + complete — every `[DEV]`/`[UI]` story that introduces user-facing + artifacts covered by this `[DOCS]` story is listed? ## Finding Format @@ -179,9 +185,9 @@ Reference section. | Severity | Meaning | Action | |----------|---------|--------| -| CRITICAL | Would cause incorrect implementation — missing requirements, contradictory acceptance criteria, broken dependency chain, scope reduction, stories referencing non-existent ICs or TCs, ICs with no implementing story, prerequisite story without justification or dependency declaration | Must fix before presenting | -| HIGH | Would cause rework — story too large to review in a single PR, missing testing commitment, integration gap between stories, `[QE]` story duplicating `[DEV]` test scope, stories missing Interface Changes or Validated by references, prerequisite story missing preserved-behavior tests | Should fix before presenting | -| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies | Fix if it adds value | +| CRITICAL | Would cause incorrect implementation — missing requirements, contradictory acceptance criteria, broken dependency chain, scope reduction, stories referencing non-existent ICs or TCs, ICs with no implementing story, prerequisite story without justification or dependency declaration, `[DOCS]` story missing contributing stories or with Documentation Inputs that omit user-facing artifacts from referenced implementation stories | Must fix before presenting | +| HIGH | Would cause rework — story too large to review in a single PR, missing testing commitment, integration gap between stories, `[QE]` story duplicating `[DEV]` test scope, stories missing Interface Changes or Validated by references, prerequisite story missing preserved-behavior tests, `[DOCS]` story with generic title or unidentified reader/task | Should fix before presenting | +| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story missing fact/assumption markers on Documentation Inputs items | Fix if it adds value | | LOW | Structural improvements — naming consistency, ordering suggestions, documentation clarity | Fix only if clearly valuable | ## Validation Rules diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index 5967524a..05bb967e 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -138,6 +138,7 @@ def _write(path: Path, content: str) -> None: Epic: Epic {epic_idx} — {epic_title} PRD Requirements: {prd_reqs} Design section: {design_section} + Contributing stories: {contributing_stories} """) # Story with code fences containing headings (issue #4) @@ -240,6 +241,7 @@ def _make_docs_story( doc_inputs: str = "**Story 1.01 — Scaffold:** output.", epic_title: str = "Building", prd_reqs: str = "FR-1", design_section: str = "§4.1", + contributing_stories: str = "Story 1.01", ) -> Path: extra_fm = "status: removed" if removed else "" content = _DOCS_STORY_TEMPLATE.format( @@ -248,6 +250,7 @@ def _make_docs_story( ac=ac, doc_scope=doc_scope, doc_inputs=doc_inputs, deps=deps, epic_title=epic_title, prd_reqs=prd_reqs, design_section=design_section, + contributing_stories=contributing_stories, ) epic_dir = stories_dir / f"epic-{epic_idx}" fp = epic_dir / f"story-{story_idx:02d}-{slug}.md" @@ -846,6 +849,7 @@ def test_docs_story_preserves_docs_sections(self) -> None: doc_scope="API ref.", doc_inputs="Story 1.01 output.", deps="Story 1.01", epic_title="Building", prd_reqs="FR-1", design_section="§4.1", + contributing_stories="Story 1.01", )) desc = sync_helper.render_story_description( sf, self.manifest, story_type="DOCS") @@ -855,6 +859,27 @@ def test_docs_story_preserves_docs_sections(self) -> None: # Ref resolved self.assertIn("EDM-201", desc) + def test_docs_story_contributing_stories_resolved(self) -> None: + """Contributing stories line in Design Reference is resolved to Jira keys.""" + sf = Path(self.tmpdir) / "story-02-docs.md" + _write(sf, _DOCS_STORY_TEMPLATE.format( + extra_fm="", epic_idx=1, story_idx=2, + title="Device rollback command reference", + role="fleet administrator", + want="find rollback command documentation", + so_that="I can safely revert device firmware", + ac="Reader can follow rollback procedure", + doc_scope="CLI rollback command reference.", + doc_inputs="**Story 1.01 — Scaffold:** rollback subcommand.", + deps="Story 1.01", epic_title="Building", + prd_reqs="FR-1", design_section="§4.1", + contributing_stories="Story 1.01", + )) + desc = sync_helper.render_story_description( + sf, self.manifest, story_type="DOCS") + # Contributing stories line should have resolved ref + self.assertIn("Contributing stories: Story EDM-201", desc) + def test_code_fences_do_not_break_sections(self) -> None: """Issue #4: headings inside code fences are not treated as sections.""" sf = Path(self.tmpdir) / "story-01-api.md" @@ -1258,6 +1283,7 @@ def test_docs_inferred_from_canonical_title_via_prepare(self) -> None: doc_scope="API reference.", doc_inputs="From Story 1.01.", deps="Story 1.01", epic_title="Building", prd_reqs="FR-1", design_section="§4.1", + contributing_stories="Story 1.01", )) # No --story-type — must infer from title out = StringIO() diff --git a/design/skills/decompose.md b/design/skills/decompose.md index c188c44b..1961c4a8 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -225,20 +225,19 @@ implements. `Validated by` lists the TCs from the testplan that validate the behavior this story delivers. These references are populated in Step 7 after stories are written. -**For `[DOCS]` stories** (see `[DOCS]` story requirements below for the -Documentation Inputs section): +**For `[DOCS]` stories** (see `[DOCS]` story quality criteria below): ```markdown -# Story {N}.{NN}: [DOCS] {title} +# Story {N}.{NN}: [DOCS] {product/feature/command — document title} -**As a** {role}, -**I want to** {capability}, -**So that** {benefit}. +**As a** {specific reader — e.g., fleet administrator, application developer}, +**I want to** {concrete task — what the reader will learn or accomplish}, +**So that** {benefit — why this documentation matters to the reader}. ## Acceptance Criteria -- [ ] {documentation outcome — what the reader can learn or do, verifiable by reading the documentation} -- [ ] {documentation outcome} +- [ ] {reader-verifiable documentation outcome — what the reader can learn or do, verifiable by reading the documentation} +- [ ] {reader-verifiable documentation outcome} ## Documentation Scope @@ -247,9 +246,10 @@ Documentation Inputs section): Those decisions belong to the documentation workflow or author who picks up the ticket. - Example: "This feature introduces a new CLI command for managing device - rollbacks. Users need to understand when to use rollback vs. revert, - the required preconditions, and how to verify success."} + Example: "This feature introduces a new CLI command (`flightctl rollback`) + for managing device rollbacks. Fleet administrators need to understand + when to use rollback vs. revert, the required preconditions, and how to + verify success."} ## Documentation Inputs @@ -268,6 +268,7 @@ Documentation Inputs section): Epic: Epic {N} — {epic title} PRD Requirements: {FR-1, NFR-1} Design section: {§4.3 API Changes, or specific subsection} +Contributing stories: {Story 1.01, Story 1.02 — DEV/UI stories whose user-facing artifacts this DOCS story covers} ``` **Naming conventions:** @@ -288,36 +289,80 @@ the work: If unsure of the correct prefix, use `[DEV]` as the default and note it for user review. -**`[DOCS]` story requirements:** `[DOCS]` stories use a different template -than other story types (see above). They replace `Implementation Guidance` -and `Testing Approach` with `Documentation Scope` and `Documentation Inputs`. - -The **Documentation Scope** section describes what the reader needs to -understand about the feature — not how to write the docs or where to put -them. Those decisions belong to the documentation author or workflow that -picks up the ticket. See the template example above for illustration. - -The **Documentation Inputs** section inventories every user-facing artifact -introduced or changed by the feature, grouped by the implementation story -that delivers it. At design time you have the complete picture of what's -new and what changed. Capture it so the documentation author (human or AI) -gets a concrete starting point — rather than reverse-engineering the -user-visible surface from code diffs after implementation. Note that -implementation may diverge from the design, so these inputs are guidance, -not authority. The documentation author should verify against the actual -implementation. - -Group items under bold story headers (e.g., **Story 1.01 — {title}:**) -followed by bullet points listing each user-facing artifact: new or changed -API fields/resources, CLI output or behavior, configuration options, status -values and their meanings, error messages, and UI elements. Give enough -detail that the writer knows what to look for in the implementation. - -Place each `[DOCS]` story after all stories it references so that all -references are resolvable during `/sync`. For per-epic `[DOCS]` stories, -place them as the last story within their own epic. For a cross-cutting -`[DOCS]` story that spans multiple epics, place it as the last story in -the last epic. +**`[DOCS]` story quality criteria:** `[DOCS]` stories use a different +template than other story types (see above). They replace `Implementation +Guidance` and `Testing Approach` with `Documentation Scope` and +`Documentation Inputs`. The criteria below define what a well-formed +`[DOCS]` story contains at design time. These same criteria are used by +the decomposition review (criterion #8), by `/revise` and `/respond` +when changes affect `[DOCS]` stories, and by `/sync` as a pre-sync +validation gate. + +1. **Searchable title.** The title must keep the `[DOCS]` prefix and + include recognizable product, feature, or command terms that make the + story findable in Jira search and backlog views — e.g., + `[DOCS] Device rollback command reference` rather than + `[DOCS] Feature documentation`. Use terms already established by the + PRD or design; do not invent identifiers or version strings that are + not yet known. + +2. **Explicit reader and task.** The persona/goal/benefit statement must + name the actual documentation reader (e.g., "fleet administrator," + "application developer") and a concrete task — not a generic role + like "user" or a process-oriented goal like "have documentation." + +3. **Reader-verifiable acceptance criteria.** Each AC must describe a + documentation outcome verifiable by reading the published + documentation — what the reader can learn, look up, or accomplish by + following the documented procedure. Do not require implementation + artifacts (merged PRs, CI status) that do not exist at design time. + +4. **Documentation Scope.** Describes what the reader needs to understand + about the feature — not how to write the docs or where to put them. + Those decisions belong to the documentation author or workflow that + picks up the ticket. See the template example above for illustration. + +5. **Complete Documentation Inputs.** Inventories every user-facing + artifact introduced or changed by the feature, grouped by the + `[DEV]` or `[UI]` implementation story that delivers it. Include only + contributing story types that are relevant — a feature with no `[UI]` + stories should not reference UI stories, and vice versa. At design + time you have the complete picture of what's new and what changed. + Capture it so the documentation author (human or AI) gets a concrete + starting point — rather than reverse-engineering the user-visible + surface from code diffs after implementation. + + Group items under bold story headers (e.g., **Story 1.01 — {title}:**) + followed by bullet points listing each user-facing artifact: new or + changed API fields/resources, CLI output or behavior, configuration + options, status values and their meanings, error messages, and UI + elements. Give enough detail that the writer knows what to look for in + the implementation. + + **Fact/assumption distinction.** Mark each input's confidence level + using the design document's source markers: + - Items derived from the design document or PRD are **planned** (no + marker needed — they are the default). + - Items that fill gaps not specified by the PRD or design must be + marked `[Assumption]`. + - Items awaiting resolution of an open question must be marked + `[Open: §9.N]` referencing the design document's open question. + + Implementation may diverge from the design, so these inputs are + guidance, not authority. The documentation author should verify + against the actual implementation. + +6. **Contributing stories reference.** The Design Reference section must + include a `Contributing stories` line listing every `[DEV]` and/or + `[UI]` story whose user-facing artifacts this `[DOCS]` story covers. + This must be consistent with the stories referenced in Documentation + Inputs. Use the canonical `Story N.NN` format. + +7. **Placement.** Place each `[DOCS]` story after all stories it + references so that all references are resolvable during `/sync`. For + per-epic `[DOCS]` stories, place them as the last story within their + own epic. For a cross-cutting `[DOCS]` story that spans multiple + epics, place it as the last story in the last epic. **Story quality checks:** - For `[DEV]` stories: does this story add a capability AND testing for that capability? For a justified behavior-preserving prerequisite, does it restructure existing code AND include tests demonstrating preserved behavior? @@ -341,12 +386,11 @@ the last epic. time and implementation time. Identifiers are robust. - If a story has more than ~8 acceptance criteria, examine whether it can be split along a natural boundary. -- For `[DOCS]` stories: does the Documentation Scope describe what the - reader needs to understand without prescribing how to write or organize - the documentation? -- For `[DOCS]` stories: does the Documentation Inputs section list every - user-facing artifact from the implementation stories, with enough detail - for a documentation author to know what to look for in the implementation? +- For `[DOCS]` stories: does the story satisfy all seven `[DOCS]` story + quality criteria above (searchable title, explicit reader/task, + reader-verifiable AC, documentation scope, complete documentation + inputs with fact/assumption distinction, contributing stories + reference, correct placement)? **Filename stability:** Story filenames (e.g., `story-01-scaffold-build-pipeline.md`) serve as idempotency keys in the sync manifest. Once stories have been synced diff --git a/design/skills/respond.md b/design/skills/respond.md index b53fa447..db9cc277 100644 --- a/design/skills/respond.md +++ b/design/skills/respond.md @@ -522,6 +522,19 @@ references a TC ID that was removed), fix it before proceeding. If the decomposition is affected, flag it and recommend `/revise` or re-running `/decompose`. +**`[DOCS]` story re-evaluation:** If decomposition artifacts exist +(`06-stories/` directory) and any design changes affected user-facing +surfaces (Interface Changes, API shapes, CLI commands, configuration +options), identify `[DOCS]` stories whose contributing `[DEV]`/`[UI]` +stories cover the changed surfaces. Re-evaluate each affected `[DOCS]` +story against the `[DOCS]` story quality criteria defined in +`decompose.md` — specifically verify that Documentation Inputs still +accurately inventory the user-facing artifacts, fact/assumption/open- +question markers are still correct (e.g., an open question resolved +during review should update the marker), and Contributing stories in +Design Reference are still complete. Apply fixes to affected `[DOCS]` +story files before proceeding. + ### Step 7: Report to User Summarize: diff --git a/design/skills/revise.md b/design/skills/revise.md index 08b6991f..7c5318af 100644 --- a/design/skills/revise.md +++ b/design/skills/revise.md @@ -129,6 +129,33 @@ After applying changes, verify: added, removed, or reassigned. - Do story dependencies still make sense? +**`[DOCS]` story re-evaluation (if any `[DOCS]` story is affected):** + +Re-evaluate every affected `[DOCS]` story against the `[DOCS]` story +quality criteria defined in `decompose.md`. A `[DOCS]` story is +affected when any of the following occurred during this revision: +- The `[DOCS]` story file itself was modified. +- A `[DEV]` or `[UI]` story listed in its `Contributing stories` or + `Documentation Inputs` was added, removed, renamed, or had its + user-facing artifacts (acceptance criteria, Implementation Guidance) + changed. +- The design's Interface Changes (§5) changed in ways that alter the + user-facing surface covered by the `[DOCS]` story. + +For each affected `[DOCS]` story, re-read the story file and verify: +1. The title still includes searchable product/feature/command terms. +2. The reader and task in the persona statement are still accurate. +3. Acceptance criteria are still reader-verifiable documentation outcomes. +4. Documentation Inputs still inventory every user-facing artifact from + all contributing stories, with correct fact/assumption/open-question + markers. If a contributing story's user-facing artifacts changed, + update the corresponding Documentation Inputs group. +5. The `Contributing stories` line in Design Reference is consistent + with the Documentation Inputs headers and includes every `[DEV]`/ + `[UI]` story whose user-facing artifacts this `[DOCS]` story covers. +6. If contributing stories were added or removed, update Dependencies + to match. + **If the testplan exists (`04-testplan.md`):** - Before modifying any test cases, record the IC and requirement mappings of any TCs that will be removed, reassigned, or have their diff --git a/design/skills/sync.md b/design/skills/sync.md index 1af4d042..145a2de4 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -247,6 +247,30 @@ note "*(orphaned — no artifact file)*" in the Local File column. If the user wants changes, recommend `/revise` to update the decomposition first — do not modify the decomposition during sync. +### Step 3b: Validate `[DOCS]` Stories Before Sync + +Before proceeding to Jira operations, validate every `[DOCS]` story that +is categorized as **new** or **changed** in the change plan. For each +such `[DOCS]` story, verify it against the `[DOCS]` story quality +criteria defined in `decompose.md`: + +1. Title includes searchable product/feature/command terms beyond the + `[DOCS]` prefix. +2. Persona/goal/benefit names an actual reader and concrete task. +3. Acceptance criteria are reader-verifiable documentation outcomes. +4. Documentation Inputs inventories user-facing artifacts from all + contributing stories with fact/assumption/open-question markers. +5. `Contributing stories` in Design Reference is consistent with + Documentation Inputs headers. + +If any `[DOCS]` story fails validation, present the findings to the +user and recommend `/revise` to fix the story before syncing. Do not +create or update a `[DOCS]` story in Jira that fails these criteria — +the story would produce a low-quality Jira ticket that misleads the +documentation author. + +Non-`[DOCS]` stories and unchanged `[DOCS]` stories skip this check. + ### Step 4: Sync Epics Process epics in three passes: create new epics, update changed epics, From 67228a4e0b66c1f9cd6c54d82620182c9547d248 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 19:23:28 +0000 Subject: [PATCH 02/12] Broaden DOCS re-evaluation triggers, unify criteria references, fix marker semantics Address review feedback on the DOCS story quality criteria change: - respond.md: broaden trigger to include direct DOCS edits, contributing story changes, and open-question resolutions (not only user-facing surface changes). Reference full seven criteria instead of partial list. - sync.md: replace five-item partial restatement with concise reference to the full criteria in decompose.md. Add indirect contributor detection so DOCS stories affected by modified contributing stories are caught even when the DOCS file itself is unchanged; recommend /revise rather than editing source during sync. - revise.md: add open-question resolution as a trigger. Reference full criteria. Preserve legitimate prerequisite dependencies when updating contributing stories. - decomposition-review.md: clarify that planned inputs (design/PRD-derived) need no marker; only assumptions and unresolved questions need markers. - decompose.md: replace product-specific command example (flightctl rollback) with neutral hypothetical wording in template illustration. Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 2 +- design/skills/decompose.md | 7 +++---- design/skills/respond.md | 28 +++++++++++++++---------- design/skills/revise.md | 32 ++++++++++++++-------------- design/skills/sync.md | 38 ++++++++++++++++++---------------- 5 files changed, 56 insertions(+), 51 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index 75484a82..50f4b0b7 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -187,7 +187,7 @@ Reference section. |----------|---------|--------| | CRITICAL | Would cause incorrect implementation — missing requirements, contradictory acceptance criteria, broken dependency chain, scope reduction, stories referencing non-existent ICs or TCs, ICs with no implementing story, prerequisite story without justification or dependency declaration, `[DOCS]` story missing contributing stories or with Documentation Inputs that omit user-facing artifacts from referenced implementation stories | Must fix before presenting | | HIGH | Would cause rework — story too large to review in a single PR, missing testing commitment, integration gap between stories, `[QE]` story duplicating `[DEV]` test scope, stories missing Interface Changes or Validated by references, prerequisite story missing preserved-behavior tests, `[DOCS]` story with generic title or unidentified reader/task | Should fix before presenting | -| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story missing fact/assumption markers on Documentation Inputs items | Fix if it adds value | +| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story missing `[Assumption]` or `[Open: §9.N]` markers on Documentation Inputs items that are not derived from the design or PRD (planned items need no marker) | Fix if it adds value | | LOW | Structural improvements — naming consistency, ordering suggestions, documentation clarity | Fix only if clearly valuable | ## Validation Rules diff --git a/design/skills/decompose.md b/design/skills/decompose.md index 1961c4a8..a423db9f 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -246,10 +246,9 @@ Step 7 after stories are written. Those decisions belong to the documentation workflow or author who picks up the ticket. - Example: "This feature introduces a new CLI command (`flightctl rollback`) - for managing device rollbacks. Fleet administrators need to understand - when to use rollback vs. revert, the required preconditions, and how to - verify success."} + Example: "This feature introduces a new CLI command for managing device + rollbacks. Operators need to understand when to use rollback vs. revert, + the required preconditions, and how to verify success."} ## Documentation Inputs diff --git a/design/skills/respond.md b/design/skills/respond.md index db9cc277..21e39e4b 100644 --- a/design/skills/respond.md +++ b/design/skills/respond.md @@ -523,17 +523,23 @@ If the decomposition is affected, flag it and recommend `/revise` or re-running `/decompose`. **`[DOCS]` story re-evaluation:** If decomposition artifacts exist -(`06-stories/` directory) and any design changes affected user-facing -surfaces (Interface Changes, API shapes, CLI commands, configuration -options), identify `[DOCS]` stories whose contributing `[DEV]`/`[UI]` -stories cover the changed surfaces. Re-evaluate each affected `[DOCS]` -story against the `[DOCS]` story quality criteria defined in -`decompose.md` — specifically verify that Documentation Inputs still -accurately inventory the user-facing artifacts, fact/assumption/open- -question markers are still correct (e.g., an open question resolved -during review should update the marker), and Contributing stories in -Design Reference are still complete. Apply fixes to affected `[DOCS]` -story files before proceeding. +(`06-stories/` directory), identify every `[DOCS]` story affected by +this respond round and re-evaluate it against the full `[DOCS]` story +quality criteria defined in `decompose.md`. A `[DOCS]` story is +affected when any of the following occurred: +- The `[DOCS]` story file itself was modified in this round. +- A `[DEV]` or `[UI]` story listed in its `Contributing stories` or + `Documentation Inputs` had its user-facing artifacts, acceptance + criteria, or Implementation Guidance changed. +- The design's Interface Changes (§5) changed in ways that alter the + user-facing surface covered by the `[DOCS]` story. +- An open question referenced in the `[DOCS]` story's Documentation + Inputs (via `[Open: §9.N]`) was resolved during this round — the + marker should be removed or updated even when the resolution does + not change API/CLI shapes. + +For each affected `[DOCS]` story, re-read the file and verify all +seven criteria. Apply fixes to affected story files before proceeding. ### Step 7: Report to User diff --git a/design/skills/revise.md b/design/skills/revise.md index 7c5318af..aebe0bca 100644 --- a/design/skills/revise.md +++ b/design/skills/revise.md @@ -131,9 +131,10 @@ After applying changes, verify: **`[DOCS]` story re-evaluation (if any `[DOCS]` story is affected):** -Re-evaluate every affected `[DOCS]` story against the `[DOCS]` story -quality criteria defined in `decompose.md`. A `[DOCS]` story is -affected when any of the following occurred during this revision: +Re-evaluate every affected `[DOCS]` story against the full `[DOCS]` +story quality criteria (all seven) defined in `decompose.md`. A +`[DOCS]` story is affected when any of the following occurred during +this revision: - The `[DOCS]` story file itself was modified. - A `[DEV]` or `[UI]` story listed in its `Contributing stories` or `Documentation Inputs` was added, removed, renamed, or had its @@ -141,20 +142,17 @@ affected when any of the following occurred during this revision: changed. - The design's Interface Changes (§5) changed in ways that alter the user-facing surface covered by the `[DOCS]` story. - -For each affected `[DOCS]` story, re-read the story file and verify: -1. The title still includes searchable product/feature/command terms. -2. The reader and task in the persona statement are still accurate. -3. Acceptance criteria are still reader-verifiable documentation outcomes. -4. Documentation Inputs still inventory every user-facing artifact from - all contributing stories, with correct fact/assumption/open-question - markers. If a contributing story's user-facing artifacts changed, - update the corresponding Documentation Inputs group. -5. The `Contributing stories` line in Design Reference is consistent - with the Documentation Inputs headers and includes every `[DEV]`/ - `[UI]` story whose user-facing artifacts this `[DOCS]` story covers. -6. If contributing stories were added or removed, update Dependencies - to match. +- An open question referenced in the `[DOCS]` story's Documentation + Inputs (via `[Open: §9.N]`) was resolved during this revision. + +For each affected `[DOCS]` story, re-read the story file and verify +all seven criteria. If a contributing story's user-facing artifacts +changed, update the corresponding Documentation Inputs group. If +contributing stories were added or removed, update both the +`Contributing stories` line in Design Reference and the Dependencies +section — but preserve any existing prerequisite dependencies (e.g., +infrastructure stories) that are not part of the contributing-story +inventory. **If the testplan exists (`04-testplan.md`):** - Before modifying any test cases, record the IC and requirement diff --git a/design/skills/sync.md b/design/skills/sync.md index 145a2de4..e38e3917 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -249,27 +249,29 @@ first — do not modify the decomposition during sync. ### Step 3b: Validate `[DOCS]` Stories Before Sync -Before proceeding to Jira operations, validate every `[DOCS]` story that -is categorized as **new** or **changed** in the change plan. For each -such `[DOCS]` story, verify it against the `[DOCS]` story quality -criteria defined in `decompose.md`: - -1. Title includes searchable product/feature/command terms beyond the - `[DOCS]` prefix. -2. Persona/goal/benefit names an actual reader and concrete task. -3. Acceptance criteria are reader-verifiable documentation outcomes. -4. Documentation Inputs inventories user-facing artifacts from all - contributing stories with fact/assumption/open-question markers. -5. `Contributing stories` in Design Reference is consistent with - Documentation Inputs headers. +Before proceeding to Jira operations, validate every `[DOCS]` story +that is categorized as **new** or **changed** in the change plan +against the full `[DOCS]` story quality criteria (all seven) defined +in `decompose.md`. + +A `[DOCS]` story whose change-plan category is **changed** because a +contributing `[DEV]`/`[UI]` story was modified (and the `[DOCS]` file +was updated by `/revise` or `/respond` to reflect the change) is +already covered. If a contributing story was modified but the `[DOCS]` +file was not yet updated, the `[DOCS]` story's content hash will not +show a change. In that case, identify affected `[DOCS]` stories by +scanning Documentation Inputs and Contributing stories references for +any story whose file hash changed since the last sync, and include +those `[DOCS]` stories in validation even though their own hash is +unchanged. If any `[DOCS]` story fails validation, present the findings to the -user and recommend `/revise` to fix the story before syncing. Do not -create or update a `[DOCS]` story in Jira that fails these criteria — -the story would produce a low-quality Jira ticket that misleads the -documentation author. +user and recommend `/revise` to fix the story before syncing — sync +must not modify story source files. Do not create or update a `[DOCS]` +story in Jira that fails these criteria. -Non-`[DOCS]` stories and unchanged `[DOCS]` stories skip this check. +Non-`[DOCS]` stories skip this check. Unchanged `[DOCS]` stories +whose contributing stories are also unchanged skip this check. ### Step 4: Sync Epics From 378e069d32d1d9a356e40f88bb801c9d5818260a Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 19:28:47 +0000 Subject: [PATCH 03/12] Condense DOCS re-evaluation blocks; broaden affected-set to scope-based detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - respond.md: condense from ~18 to ~9 lines, eliminating the token-budget FAIL (~6020 → ~5904). Affected-set now covers newly relevant DEV/UI contributors, reader/scope/AC/assumption changes in the design, and resolved open-question markers — not only already-listed contributors. - revise.md: same coherent (a-d) affected-set rule; covers new, removed, or renamed contributors whose user-facing surface falls within the DOCS story's scope, even when references are not yet updated. - sync.md: scope-based detection using the change plan's new/changed/removed items, not only existing referenced file hashes. Preserves no-source-edit invariant. Co-Authored-By: Claude Opus 4.6 --- design/skills/respond.md | 25 ++++++++----------------- design/skills/revise.md | 37 +++++++++++++++---------------------- design/skills/sync.md | 33 +++++++++++---------------------- 3 files changed, 34 insertions(+), 61 deletions(-) diff --git a/design/skills/respond.md b/design/skills/respond.md index 21e39e4b..b05021e8 100644 --- a/design/skills/respond.md +++ b/design/skills/respond.md @@ -523,23 +523,14 @@ If the decomposition is affected, flag it and recommend `/revise` or re-running `/decompose`. **`[DOCS]` story re-evaluation:** If decomposition artifacts exist -(`06-stories/` directory), identify every `[DOCS]` story affected by -this respond round and re-evaluate it against the full `[DOCS]` story -quality criteria defined in `decompose.md`. A `[DOCS]` story is -affected when any of the following occurred: -- The `[DOCS]` story file itself was modified in this round. -- A `[DEV]` or `[UI]` story listed in its `Contributing stories` or - `Documentation Inputs` had its user-facing artifacts, acceptance - criteria, or Implementation Guidance changed. -- The design's Interface Changes (§5) changed in ways that alter the - user-facing surface covered by the `[DOCS]` story. -- An open question referenced in the `[DOCS]` story's Documentation - Inputs (via `[Open: §9.N]`) was resolved during this round — the - marker should be removed or updated even when the resolution does - not change API/CLI shapes. - -For each affected `[DOCS]` story, re-read the file and verify all -seven criteria. Apply fixes to affected story files before proceeding. +(`06-stories/`), re-evaluate every affected `[DOCS]` story against +all seven criteria in `decompose.md`. A `[DOCS]` story is affected +when this round changed: (a) the `[DOCS]` file itself, (b) a listed +or newly relevant `[DEV]`/`[UI]` contributor's artifacts, (c) design +content that alters the story's reader, scope, AC, assumptions, or +open-question markers — including resolved `[Open: §9.N]` items even +without API/CLI shape changes, or (d) ICs covered by the story. Apply +fixes to affected files before proceeding. ### Step 7: Report to User diff --git a/design/skills/revise.md b/design/skills/revise.md index aebe0bca..950061ec 100644 --- a/design/skills/revise.md +++ b/design/skills/revise.md @@ -131,28 +131,21 @@ After applying changes, verify: **`[DOCS]` story re-evaluation (if any `[DOCS]` story is affected):** -Re-evaluate every affected `[DOCS]` story against the full `[DOCS]` -story quality criteria (all seven) defined in `decompose.md`. A -`[DOCS]` story is affected when any of the following occurred during -this revision: -- The `[DOCS]` story file itself was modified. -- A `[DEV]` or `[UI]` story listed in its `Contributing stories` or - `Documentation Inputs` was added, removed, renamed, or had its - user-facing artifacts (acceptance criteria, Implementation Guidance) - changed. -- The design's Interface Changes (§5) changed in ways that alter the - user-facing surface covered by the `[DOCS]` story. -- An open question referenced in the `[DOCS]` story's Documentation - Inputs (via `[Open: §9.N]`) was resolved during this revision. - -For each affected `[DOCS]` story, re-read the story file and verify -all seven criteria. If a contributing story's user-facing artifacts -changed, update the corresponding Documentation Inputs group. If -contributing stories were added or removed, update both the -`Contributing stories` line in Design Reference and the Dependencies -section — but preserve any existing prerequisite dependencies (e.g., -infrastructure stories) that are not part of the contributing-story -inventory. +Re-evaluate every affected `[DOCS]` story against all seven criteria +in `decompose.md`. A `[DOCS]` story is affected when this revision +changed: (a) the `[DOCS]` file itself, (b) a listed or newly relevant +`[DEV]`/`[UI]` contributor's artifacts — including new, removed, or +renamed stories whose user-facing surface falls within the `[DOCS]` +story's scope, (c) design content that alters the story's reader, +scope, AC, assumptions, or open-question markers — including resolved +`[Open: §9.N]` items even without API/CLI shape changes, or (d) ICs +covered by the story. + +For each affected `[DOCS]` story, re-read the file and update +Documentation Inputs, Contributing stories, and Dependencies to +reflect the current contributor set — but preserve any existing +prerequisite dependencies (e.g., infrastructure stories) that are +not part of the contributing-story inventory. **If the testplan exists (`04-testplan.md`):** - Before modifying any test cases, record the IC and requirement diff --git a/design/skills/sync.md b/design/skills/sync.md index e38e3917..4a84c937 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -250,28 +250,17 @@ first — do not modify the decomposition during sync. ### Step 3b: Validate `[DOCS]` Stories Before Sync Before proceeding to Jira operations, validate every `[DOCS]` story -that is categorized as **new** or **changed** in the change plan -against the full `[DOCS]` story quality criteria (all seven) defined -in `decompose.md`. - -A `[DOCS]` story whose change-plan category is **changed** because a -contributing `[DEV]`/`[UI]` story was modified (and the `[DOCS]` file -was updated by `/revise` or `/respond` to reflect the change) is -already covered. If a contributing story was modified but the `[DOCS]` -file was not yet updated, the `[DOCS]` story's content hash will not -show a change. In that case, identify affected `[DOCS]` stories by -scanning Documentation Inputs and Contributing stories references for -any story whose file hash changed since the last sync, and include -those `[DOCS]` stories in validation even though their own hash is -unchanged. - -If any `[DOCS]` story fails validation, present the findings to the -user and recommend `/revise` to fix the story before syncing — sync -must not modify story source files. Do not create or update a `[DOCS]` -story in Jira that fails these criteria. - -Non-`[DOCS]` stories skip this check. Unchanged `[DOCS]` stories -whose contributing stories are also unchanged skip this check. +that is **new** or **changed** in the change plan against all seven +criteria in `decompose.md`. Also include any `[DOCS]` story whose +own hash is unchanged but whose contributor set changed: a listed +contributor was modified, removed, or renamed, or a new/changed +`[DEV]`/`[UI]` story's user-facing surface falls within the `[DOCS]` +story's documented scope. Determine this from the change plan's +new/changed/removed items, not only from existing references. + +If any `[DOCS]` story fails validation, present the findings and +recommend `/revise` — sync must not modify story source files. Do +not create or update a failing `[DOCS]` story in Jira. ### Step 4: Sync Epics From 8024e155787fb0795b0d47c6c5e215ee2d06312e Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 20:01:27 +0000 Subject: [PATCH 04/12] Address PR review: renumber sync steps, add provenance check, fix dependencies, placement, reopened R1: Promote sync Step 3b to Step 4 and renumber old Steps 4-7 to 5-8, including all nested sub-step labels (5a/b/c, 6a/b/c) and ~50 prose cross-references. Eliminates the sub-step numbering warning. R2: Split provenance responsibility between decomposer and reviewer. Remove impossible demand on independent reviewer (which lacks the design document) to verify unmarked input provenance. Add explicit decomposer-side provenance check (new Step 13) after review rounds to verify unmarked Documentation Inputs against design/PRD. R3: Add dependency reconciliation to respond.md DOCS re-evaluation, matching revise.md's pattern: add new contributor dependencies, remove obsolete entries, preserve unrelated prerequisites. R4: Add post-sync placement exemption to criterion 7 in decompose.md and sync Step 4 gate. Synced DOCS stories retain immutable filenames when later contributors are appended; sync creates new stories before updating existing ones, ensuring Jira key availability. R5: Include reopened DOCS stories and reopened DEV/UI contributors in affected-set evaluation across sync Step 4, revise, and respond. Update test fixtures to match renumbered steps (205 tests pass). Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 31 +++--- design/scripts/test_sync_helper.py | 150 ++++++++++++++--------------- design/skills/decompose.md | 32 +++++- design/skills/respond.md | 9 +- design/skills/revise.md | 4 +- design/skills/sync.md | 82 +++++++++------- 6 files changed, 177 insertions(+), 131 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index 50f4b0b7..2ac925ed 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -144,21 +144,26 @@ impact: as gaps. Do not reject a legitimate prerequisite for adding no new feature behavior — that is its nature. -8. **Documentation** — Do `[DOCS]` stories satisfy all seven `[DOCS]` - story quality criteria from the decompose skill? Specifically: does - the title include searchable product/feature/command terms (not just - `[DOCS] Feature documentation`)? Does the persona/goal/benefit name - the actual reader and a concrete task? Are acceptance criteria - reader-verifiable documentation outcomes (not implementation - artifacts)? Does the Documentation Scope describe what the reader - needs to understand without prescribing how to write or organize the - documentation? Does the Documentation Inputs section inventory all - user-facing artifacts from the contributing `[DEV]`/`[UI]` stories, - with fact/assumption/open-question distinction? Does the Design +8. **Documentation** — Do `[DOCS]` stories satisfy the `[DOCS]` story + quality criteria from the decompose skill that the reviewer can + evaluate from its available inputs (PRD and decomposition artifacts)? + Specifically: does the title include searchable product/feature/ + command terms (not just `[DOCS] Feature documentation`)? Does the + persona/goal/benefit name the actual reader and a concrete task? Are + acceptance criteria reader-verifiable documentation outcomes (not + implementation artifacts)? Does the Documentation Scope describe what + the reader needs to understand without prescribing how to write or + organize the documentation? Does the Documentation Inputs section + inventory user-facing artifacts from the contributing `[DEV]`/`[UI]` + stories? Are `[Assumption]` and `[Open: §9.N]` markers present on + items that appear speculative or unresolved? Does the Design Reference include a `Contributing stories` line consistent with the Documentation Inputs headers? Are the contributing story references - complete — every `[DEV]`/`[UI]` story that introduces user-facing - artifacts covered by this `[DOCS]` story is listed? + complete? Note: the reviewer cannot verify whether unmarked + Documentation Inputs items are truly derived from the design document + or PRD — that provenance check belongs to the decomposer (see + `/decompose` Step 13). Flag only items that appear speculative from + the PRD and artifact context alone. ## Finding Format diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index 05bb967e..9eb3c408 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -2622,7 +2622,7 @@ def test_genuinely_new_story_different_index_ok(self) -> None: class TestCascadeChildClosurePermissions(_TmpDirMixin, unittest.TestCase): """Verify that detect-changes marks active children of a removed epic as 'removed' (cascade), which is the prerequisite for the approved - parent-close cascade in Step 4c closing those children in Jira.""" + parent-close cascade in Step 5c closing those children in Jira.""" def setUp(self) -> None: super().setUp() @@ -2852,7 +2852,7 @@ def test_metacharacter_paths_no_execution(self) -> None: class TestCascadeGateContract(unittest.TestCase): """Assert sync.md Steps 4c/5c document hash-before-transition ordering - and that Step 4c gates the parent close on all child successes. + and that Step 5c gates the parent close on all child successes. These are contract tests against the ACTUAL Markdown sections/order, not a test-only invented executor. They verify the documented @@ -2860,143 +2860,143 @@ class TestCascadeGateContract(unittest.TestCase): """ _sync_md: str - _step4c: str _step5c: str - _step5_refresh: str + _step6c: str + _step6_refresh: str @classmethod def setUpClass(cls) -> None: import re sync_md_path = Path(__file__).resolve().parent.parent / "skills" / "sync.md" cls._sync_md = sync_md_path.read_text(encoding="utf-8") - m4c = re.search(r"#### 4c:.*?(?=####|### )", cls._sync_md, re.DOTALL) - assert m4c, "Step 4c section not found" - cls._step4c = m4c.group() - m5c = re.search(r"#### 5c:.*?(?=####|### )", cls._sync_md, re.DOTALL) - assert m5c, "Step 5c section not found" - cls._step5c = m5c.group() - m5 = re.search(r"### Step 5:.*?(?=#### 5a:)", cls._sync_md, re.DOTALL) - assert m5, "Step 5 plan refresh not found" - cls._step5_refresh = m5.group() - - # -- Step 4c: child hash before child transition --- - - def test_step4c_child_hash_before_transition(self) -> None: - """4c child closure: hash computed before Jira transition.""" - h = self._step4c.find("Compute the hash first") - t = self._step4c.find("Transition the child story") + m4c = re.search(r"#### 5c:.*?(?=####|### )", cls._sync_md, re.DOTALL) + assert m4c, "Step 6c section not found" + cls._step5c = m4c.group() + m5c = re.search(r"#### 6c:.*?(?=####|### )", cls._sync_md, re.DOTALL) + assert m5c, "Step 6c section not found" + cls._step6c = m5c.group() + m5 = re.search(r"### Step 6:.*?(?=#### 6a:)", cls._sync_md, re.DOTALL) + assert m5, "Step 6 plan refresh not found" + cls._step6_refresh = m5.group() + + # -- Step 5c: child hash before child transition --- + + def test_step6c_child_hash_before_transition(self) -> None: + """6c child closure: hash computed before Jira transition.""" + h = self._step5c.find("Compute the hash first") + t = self._step5c.find("Transition the child story") self.assertGreater(h, -1, "must mention computing child hash") self.assertGreater(t, -1, "must mention child transition") self.assertLess(h, t, "child hash must precede child transition") - def test_step4c_epic_hash_before_transition(self) -> None: - """4c epic closure: hash computed before Jira transition.""" - h = self._step4c.find("compute the epic hash") - t = self._step4c.find("Transition the epic to Done") + def test_step6c_epic_hash_before_transition(self) -> None: + """6c epic closure: hash computed before Jira transition.""" + h = self._step5c.find("compute the epic hash") + t = self._step5c.find("Transition the epic to Done") self.assertGreater(h, -1, "must mention computing epic hash") self.assertGreater(t, -1, "must mention epic transition") self.assertLess(h, t, "epic hash must precede epic transition") - # -- Step 4c: parent gated on all children --- + # -- Step 5c: parent gated on all children --- - def test_step4c_parent_gated_on_all_children(self) -> None: + def test_step6c_parent_gated_on_all_children(self) -> None: """4c: parent close requires ALL child closures and checkpoints.""" self.assertIn("child closures and their checkpoints have succeeded", - self._step4c) + self._step5c) - def test_step4c_stop_cascade_on_failure(self) -> None: + def test_step6c_stop_cascade_on_failure(self) -> None: """4c: cascade stops immediately on any child failure.""" import re self.assertTrue( - re.search(r"stop the cascade\s+immediately", self._step4c), + re.search(r"stop the cascade\s+immediately", self._step5c), "must stop cascade immediately on child failure") - def test_step4c_no_skip_child_then_close_parent(self) -> None: + def test_step6c_no_skip_child_then_close_parent(self) -> None: """4c: explicitly forbids skipping a child then closing parent.""" import re self.assertTrue( - re.search(r"Never\s+skip\s+a failing child", self._step4c), + re.search(r"Never\s+skip\s+a failing child", self._step5c), "must forbid skip-child-close-parent") - def test_step4c_offers_retry_or_defer(self) -> None: + def test_step6c_offers_retry_or_defer(self) -> None: """4c: pre-transition failure offers retry or defer.""" import re self.assertTrue( - re.search(r"retry after\s+remediation", self._step4c), + re.search(r"retry after\s+remediation", self._step5c), "must offer retry after remediation") - self.assertIn("defer the entire parent", self._step4c) + self.assertIn("defer the entire parent", self._step5c) - def test_step4c_preserves_successful_checkpoints(self) -> None: + def test_step6c_preserves_successful_checkpoints(self) -> None: """4c: already-successful child checkpoints preserved on failure.""" import re self.assertTrue( re.search(r"Preserve all\s+already-successful\s+child checkpoints", - self._step4c), + self._step5c), "must preserve completed child checkpoints") - def test_step4c_distinguishes_pre_and_post_failure(self) -> None: + def test_step6c_distinguishes_pre_and_post_failure(self) -> None: """4c: distinguishes pre-transition from post-success checkpoint failure, referencing the shared close-checkpoint recovery rule.""" - self.assertIn("Pre-transition failure", self._step4c) - self.assertIn("Post-success checkpoint failure", self._step4c) - self.assertIn("close-checkpoint", self._step4c) + self.assertIn("Pre-transition failure", self._step5c) + self.assertIn("Post-success checkpoint failure", self._step5c) + self.assertIn("close-checkpoint", self._step5c) - def test_step4c_uncertain_outcome_requires_verification(self) -> None: + def test_step6c_uncertain_outcome_requires_verification(self) -> None: """4c: uncertain Jira outcome (timeout/5xx) must be verified before retrying, not assumed non-mutating.""" import re self.assertTrue( re.search(r"uncertain.*verify.*current status", - self._step4c, re.DOTALL), + self._step5c, re.DOTALL), "must require verification for uncertain Jira outcomes") - def test_step4c_resume_manual_reconciliation(self) -> None: + def test_step6c_resume_manual_reconciliation(self) -> None: """4c: resume stops for manual reconciliation when captured state is lost (forced abort/new run).""" - self.assertIn("stop for manual reconciliation", self._step4c) + self.assertIn("stop for manual reconciliation", self._step5c) - # -- Step 5c: hash before transition --- + # -- Step 6c: hash before transition --- - def test_step5c_hash_before_transition(self) -> None: - """5c story close: hash computed before transition.""" - h = self._step5c.find("Compute the hash before the transition") - t = self._step5c.find("Transition the story to Done") + def test_step6c_hash_before_transition(self) -> None: + """6c story close: hash computed before transition.""" + h = self._step6c.find("Compute the hash before the transition") + t = self._step6c.find("Transition the story to Done") self.assertGreater(h, -1, "must mention computing hash first") self.assertGreater(t, -1, "must mention story transition") self.assertLess(h, t, "hash must precede transition") - def test_step5c_checkpoint_uses_saved_hash(self) -> None: + def test_step6c_checkpoint_uses_saved_hash(self) -> None: """5c: checkpoint references saved hash from step 1.""" - self.assertIn("hash from step 1", self._step5c) + self.assertIn("hash from step 1", self._step6c) - # -- Step 4c: reporting distinguishes child states --- + # -- Step 5c: reporting distinguishes child states --- - def test_step4c_report_distinguishes_child_states(self) -> None: + def test_step6c_report_distinguishes_child_states(self) -> None: """4c: failure report distinguishes complete, uncheckpointed, and unattempted children.""" import re - self.assertIn("closed and checkpointed (complete)", self._step4c) + self.assertIn("closed and checkpointed (complete)", self._step5c) self.assertTrue( - re.search(r"Jira-closed but\s+uncheckpointed", self._step4c), + re.search(r"Jira-closed but\s+uncheckpointed", self._step5c), "must mention Jira-closed but uncheckpointed") - self.assertIn("unattempted", self._step4c) + self.assertIn("unattempted", self._step5c) - def test_step4c_no_rollback_implication(self) -> None: + def test_step6c_no_rollback_implication(self) -> None: """4c: does not imply already-closed children are rolled back.""" import re self.assertTrue( re.search(r"not imply that already-closed children\s+will be rolled back", - self._step4c), + self._step5c), "must not imply rollback of closed children") - # -- Step 4c: child checkpoint references helper reference --- + # -- Step 5c: child checkpoint references helper reference --- - def test_step4c_child_checkpoint_references_helper(self) -> None: - """4c child checkpoint step references the helper reference.""" - self.assertIn("helper reference", self._step4c) + def test_step6c_child_checkpoint_references_helper(self) -> None: + """6c child checkpoint step references the helper reference.""" + self.assertIn("helper reference", self._step5c) # The step must mention the required story-specific args - self.assertIn("--item-type story", self._step4c) - self.assertIn("--epic-file", self._step4c) + self.assertIn("--item-type story", self._step5c) + self.assertIn("--epic-file", self._step5c) def test_helper_reference_child_close_has_all_args(self) -> None: """Helper reference child-close recipe has all required args.""" @@ -3138,26 +3138,26 @@ def test_recovery_rule_uncertain_outcome_verification(self) -> None: def test_all_close_paths_reference_recovery_rule(self) -> None: """All three close paths (child, epic, 5c) reference the shared close-checkpoint recovery rule.""" - self.assertIn("close-checkpoint", self._step4c) + self.assertIn("close-checkpoint", self._step5c) # Epic close failure handler - epic_fail_pos = self._step4c.find("If epic close fails") + epic_fail_pos = self._step5c.find("If epic close fails") self.assertGreater(epic_fail_pos, -1) - epic_fail_text = self._step4c[epic_fail_pos:] + epic_fail_text = self._step5c[epic_fail_pos:] self.assertIn("close-checkpoint", epic_fail_text) # Step 5c - self.assertIn("close-checkpoint", self._step5c) + self.assertIn("close-checkpoint", self._step6c) - # -- Step 5 plan refresh: deferred cascades --- + # -- Step 6 plan refresh: deferred cascades --- def test_step5_deferred_cascade_not_retried(self) -> None: - """Step 5 refresh: deferred cascades are not silently retried.""" - self.assertIn("deferred in Step 4c", self._step5_refresh) - self.assertIn("do not retry closing", self._step5_refresh) + """Step 6 refresh: deferred cascades are not silently retried.""" + self.assertIn("deferred in Step 5c", self._step6_refresh) + self.assertIn("do not retry closing", self._step6_refresh) def test_step5_deferral_covers_checkpoint_failure(self) -> None: - """Step 5 refresh: deferral clause covers Jira-closed-but- + """Step 6 refresh: deferral clause covers Jira-closed-but- uncheckpointed state, not just open children.""" - self.assertIn("Jira-closed but uncheckpointed", self._step5_refresh) + self.assertIn("Jira-closed but uncheckpointed", self._step6_refresh) # --------------------------------------------------------------------------- @@ -3313,7 +3313,7 @@ def test_orphan_empty_hash_not_fabricated(self) -> None: with self.assertRaises(FileNotFoundError): sync_helper.compute_file_hash(self.sf2) - # Orphan close uses empty hash per Step 4c/5c + # Orphan close uses empty hash per Step 5c/6c sync_helper.update_manifest( self.mf, "close", "story-02-pipe.md", "EDM-202", "", item_type="story", diff --git a/design/skills/decompose.md b/design/skills/decompose.md index a423db9f..c3c68341 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -361,7 +361,13 @@ validation gate. references so that all references are resolvable during `/sync`. For per-epic `[DOCS]` stories, place them as the last story within their own epic. For a cross-cutting `[DOCS]` story that spans multiple - epics, place it as the last story in the last epic. + epics, place it as the last story in the last epic. **Post-sync + exemption:** once a `[DOCS]` story has been synced (its filename is + in the manifest), it retains its immutable filename and number even + when a later contributor is appended with a higher story number. + This is safe because `/sync` creates new stories before updating + existing ones, so the contributor's Jira key is in the manifest + when the `[DOCS]` story's description is rendered. **Story quality checks:** - For `[DEV]` stories: does this story add a capability AND testing for that capability? For a justified behavior-preserving prerequisite, does it restructure existing code AND include tests demonstrating preserved behavior? @@ -616,7 +622,25 @@ If Step 11 produced changes to the decomposition artifacts: If no fixes were needed in Step 11, the review passes immediately. -### Step 13: Report Review Summary +### Step 13: Verify `[DOCS]` Input Provenance + +The independent reviewer cannot verify whether unmarked Documentation +Inputs items are truly derived from the design document or PRD — it +intentionally lacks the design document. After the review rounds +complete, verify this as the decomposer: + +For each `[DOCS]` story, check every unmarked item in Documentation +Inputs against the design document (`03-design.md`) and the PRD. An +item that traces to a specific design section or PRD requirement is +correctly unmarked (planned). An item that fills a gap not specified +by either document must be marked `[Assumption]`. An item that +depends on an unresolved open question must be marked +`[Open: §9.N]` — use the existing open-question identifier from the +design document; do not invent new identifiers for this purpose. + +Fix any incorrect markers before proceeding to Step 14. + +### Step 14: Report Review Summary ```markdown ## Decomposition Review Summary @@ -631,7 +655,7 @@ If no fixes were needed in Step 11, the review passes immediately. list them with their file, section, and issue description.} ``` -### Step 14: Present to User +### Step 15: Present to User Present the decomposition and highlight: - Number of epics and stories @@ -639,7 +663,7 @@ Present the decomposition and highlight: - Any coverage gaps - Stories that might need size adjustment (too large or too small) - Any assumptions or judgment calls in the decomposition -- Decomposition review summary (from Step 13) +- Decomposition review summary (from Step 14) If the decomposition review gate reported FLAG, present the unfixed CRITICAL/HIGH findings and ask the user to decide how to handle them. diff --git a/design/skills/respond.md b/design/skills/respond.md index b05021e8..cb58b72e 100644 --- a/design/skills/respond.md +++ b/design/skills/respond.md @@ -529,8 +529,13 @@ when this round changed: (a) the `[DOCS]` file itself, (b) a listed or newly relevant `[DEV]`/`[UI]` contributor's artifacts, (c) design content that alters the story's reader, scope, AC, assumptions, or open-question markers — including resolved `[Open: §9.N]` items even -without API/CLI shape changes, or (d) ICs covered by the story. Apply -fixes to affected files before proceeding. +without API/CLI shape changes, or (d) ICs covered by the story. + +When contributors change, update Documentation Inputs, Contributing +stories, and Dependencies to reflect the current contributor set — +add dependencies for newly required contributors, remove obsolete +contribution-only entries, but preserve legitimate unrelated +prerequisite dependencies. Apply all fixes before proceeding. ### Step 7: Report to User diff --git a/design/skills/revise.md b/design/skills/revise.md index 950061ec..8806e944 100644 --- a/design/skills/revise.md +++ b/design/skills/revise.md @@ -134,8 +134,8 @@ After applying changes, verify: Re-evaluate every affected `[DOCS]` story against all seven criteria in `decompose.md`. A `[DOCS]` story is affected when this revision changed: (a) the `[DOCS]` file itself, (b) a listed or newly relevant -`[DEV]`/`[UI]` contributor's artifacts — including new, removed, or -renamed stories whose user-facing surface falls within the `[DOCS]` +`[DEV]`/`[UI]` contributor's artifacts — including new, removed, +renamed, or reopened stories whose user-facing surface falls within the `[DOCS]` story's scope, (c) design content that alters the story's reader, scope, AC, assumptions, or open-question markers — including resolved `[Open: §9.N]` items even without API/CLI shape changes, or (d) ICs diff --git a/design/skills/sync.md b/design/skills/sync.md index 4a84c937..3dce5e1a 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -41,7 +41,7 @@ your responsibility. - **Manifest gate.** Before proceeding past Step 1, you **must** state aloud to the user what the manifest contains (or that none exists). This is mandatory — do not silently skip this acknowledgment. If a manifest exists and you cannot read it, stop and report the error. - **Jira-side duplicate check.** Before creating each epic or story, query Jira for existing children under the parent with a matching summary. If a match is found, stop and present the match to the user — do not create a duplicate. This is a safety net independent of the manifest. - **Jira hierarchy is fixed.** Feature → Epic → Story. Every epic's parent is the Feature issue; every story's parent is its epic. No operation may violate this hierarchy — no re-parenting, no creating issues outside this structure, no closing a Feature. -- **Sync-owned fields.** Sync owns: summary, description, T-shirt size (epics only), parent link (creation only), and issue links (dependencies). These are set from artifact content on creation and overwritten on updates (except parent link). Sync never touches: assignee, sprint, comments, labels, or any other Jira-managed field. Status transitions are permitted **only** for gated lifecycle operations: reopening a previously closed item (Step 4b/5b), closing a removed or user-selected orphaned item (Step 4c/5c), and closing active child stories as part of an approved parent epic closure cascade (Step 4c — the user's approval of the parent close covers its children). No other status changes are allowed. +- **Sync-owned fields.** Sync owns: summary, description, T-shirt size (epics only), parent link (creation only), and issue links (dependencies). These are set from artifact content on creation and overwritten on updates (except parent link). Sync never touches: assignee, sprint, comments, labels, or any other Jira-managed field. Status transitions are permitted **only** for gated lifecycle operations: reopening a previously closed item (Step 5b/6b), closing a removed or user-selected orphaned item (Step 5c/6c), and closing active child stories as part of an approved parent epic closure cascade (Step 5c — the user's approval of the parent close covers its children). No other status changes are allowed. - **Logical deletion via marker.** Stories and epics are closed (not deleted) when their artifact file contains `status: removed` in the YAML frontmatter. The file stays on disk so the manifest can still track it. - **Link to source.** Every Jira issue description references the design document. - **Jira-native references.** Local identifiers (`Story 1.01`, `Epic 1`) have meaning only within the `.artifacts/` directory. When constructing Jira issue descriptions, resolve all local references (in Dependencies, Documentation Inputs, Design Reference, and any other cross-references) to Jira issue keys using the sync manifest. Jira is the source of truth — readers of a Jira issue should never need to decode a local artifact numbering scheme. @@ -247,27 +247,39 @@ note "*(orphaned — no artifact file)*" in the Local File column. If the user wants changes, recommend `/revise` to update the decomposition first — do not modify the decomposition during sync. -### Step 3b: Validate `[DOCS]` Stories Before Sync +### Step 4: Validate `[DOCS]` Stories Before Sync Before proceeding to Jira operations, validate every `[DOCS]` story -that is **new** or **changed** in the change plan against all seven -criteria in `decompose.md`. Also include any `[DOCS]` story whose -own hash is unchanged but whose contributor set changed: a listed -contributor was modified, removed, or renamed, or a new/changed -`[DEV]`/`[UI]` story's user-facing surface falls within the `[DOCS]` -story's documented scope. Determine this from the change plan's -new/changed/removed items, not only from existing references. +that is **new**, **changed**, or **reopened** in the change plan +against all seven criteria in `decompose.md`. Also include any +`[DOCS]` story whose own hash is unchanged but whose contributor set +changed: a listed contributor was modified, removed, renamed, or +reopened, or a new/changed/reopened `[DEV]`/`[UI]` story's user-facing +surface falls within the `[DOCS]` story's documented scope. Determine +this from the change plan's full item list (new, changed, removed, +reopened), not only from existing references. + +When evaluating criterion 7 (placement), synced `[DOCS]` stories +retain their immutable filename and number. If a contributor is +appended with a higher story number after the `[DOCS]` story was +synced, the `[DOCS]` story may numerically precede it. This does not +fail the gate because sync processes stories in numeric order within +each epic: new contributors are created in Step 6a before existing +`[DOCS]` stories are updated in Step 6b, so the contributor's Jira +key is available in the manifest at `[DOCS]` rendering time. The +same applies to cross-cutting `[DOCS]` stories whose last-epic +contributors are appended after the `[DOCS]` story number. If any `[DOCS]` story fails validation, present the findings and recommend `/revise` — sync must not modify story source files. Do not create or update a failing `[DOCS]` story in Jira. -### Step 4: Sync Epics +### Step 5: Sync Epics Process epics in three passes: create new epics, update changed epics, close removed epics. Use the change plan items filtered by `type: "epic"`. -#### 4a: Create New Epics +#### 5a: Create New Epics **Pre-creation duplicate check (per epic).** Before creating each epic, search Jira for existing children of {feature-key} whose summary matches @@ -328,7 +340,7 @@ failed, including the error. Offer to: (a) retry the failed epic, or (b) skip it and continue with the remaining epics. Do not silently continue past a failure. -#### 4b: Update Changed Epics +#### 5b: Update Changed Epics For each epic categorized as **Changed** or **Reopened**, prepare the updated description and hash using the epic `prepare` recipe from the @@ -346,8 +358,8 @@ Update only the sync-owned fields. Do not touch assignee or other Jira-managed fields. Status transitions are permitted only for lifecycle operations: transition **Reopened** items to an open status before updating content, and transition **Removed** or user-selected -**Orphaned** items to a closed status during the close passes (Steps 4c, -5c). Do not change status for any other reason. +**Orphaned** items to a closed status during the close passes (Steps 5c, +6c). Do not change status for any other reason. After updating, record the new hash using `update-manifest --operation update` from the helper reference. @@ -355,7 +367,7 @@ update` from the helper reference. **If update fails:** Report the error with the Jira key and offer to retry or skip. -#### 4c: Close Removed and Selected Orphaned Epics +#### 5c: Close Removed and Selected Orphaned Epics For each epic categorized as **Removed** (file has `status: removed`) or **Orphaned** where the user chose to close it in Step 1: @@ -429,10 +441,10 @@ Present all epic operations to the user: **Wait for user confirmation before syncing stories.** -### Step 5: Sync Stories +### Step 6: Sync Stories **Refresh the change plan** before processing stories. Epic operations -in Step 4 may have changed the manifest (e.g., closing an epic closes +in Step 5 may have changed the manifest (e.g., closing an epic closes its children, reopening an epic enables its children). Re-run `detect-changes` to get an updated plan that reflects the current manifest state: @@ -443,29 +455,29 @@ STORY_PLAN=$(python3 "$SYNC_HELPER" detect-changes .artifacts/design/{issue-key} Use the refreshed plan's `type: "story"` items for story operations. This ensures: -- Children already closed by cascaded epic closure (Step 4c) are not +- Children already closed by cascaded epic closure (Step 5c) are not re-closed - Stories under a reopened epic are correctly categorized -- User orphan choices from Step 1 that were not acted on in Step 4 are +- User orphan choices from Step 1 that were not acted on in Step 5 are preserved (orphan entries the user chose to ignore remain in the manifest and will still appear as orphaned in the refreshed plan) If a parent epic's create or reopen operation failed/was skipped in -Step 4, do not create, update, or reopen stories under that epic — +Step 5, do not create, update, or reopen stories under that epic — skip them and report which stories were skipped due to the parent failure. -If an epic's close cascade was deferred in Step 4c (the parent was +If an epic's close cascade was deferred in Step 5c (the parent was left unclosed because a child closure or checkpoint failed — whether remaining children are open, Jira-closed but uncheckpointed, or unattempted), do not retry closing those remaining children here. They remain in their current manifest state. Report them as deferred and inform the user that the parent cascade must be retried from -Step 4c after remediation. +Step 5c after remediation. Process stories per epic, in the same three passes: create, update, close. -#### 5a: Create New Stories +#### 6a: Create New Stories **Pre-creation duplicate check (per story).** Before creating each story, search Jira for existing children of the epic whose summary matches the @@ -477,7 +489,7 @@ parent = {epic-jira-key} AND issuetype = Story AND summary ~ "{story title}" If the query returns one or more matching issues, **do not create the story.** Instead, stop and present the match to the user with the same -options as in Step 4a (skip and record, create anyway, or stop entirely). +options as in Step 5a (skip and record, create anyway, or stop entirely). Wait for the user's choice before continuing. For each new story, prepare the hash and description using the story @@ -489,7 +501,7 @@ Create the Jira issue using fields from `PREP`: - **Type:** Story - **Project:** {project key} -- **Parent:** {epic jira key} (from Step 4) — **set via the `fields` parameter: `{"parent": {"key": "{epic jira key}"}}`** +- **Parent:** {epic jira key} (from Step 5) — **set via the `fields` parameter: `{"parent": {"key": "{epic jira key}"}}`** - **Summary:** `summary` from PREP (e.g. "[DEV] Scaffold Build Pipeline" — without "Story 1.01:" prefix) - **Description:** `description` from PREP @@ -559,7 +571,7 @@ successfully and which one failed, including the error. Offer to: stories. The manifest already reflects all successfully created items, so re-running `/sync` later will only attempt uncreated items. -#### 5b: Update Changed Stories +#### 6b: Update Changed Stories For each story categorized as **Changed** or **Reopened**, prepare the updated description and hash using the story `prepare` recipe from the @@ -570,18 +582,18 @@ Update the Jira issue using fields from `PREP`: - **Summary:** `summary` from PREP - **Description:** `description` from PREP - **Issue links:** read the Jira issue's current issue links and filter - to only those matching the link type selected in Step 5a (either + to only those matching the link type selected in Step 6a (either "Dependency" or "Blocks", whichever was used during creation). Compare these filtered links against the artifact file's dependency list. Remove links whose dependency stories no longer appear in the artifact and create new links for added dependencies, using the same - link type and direction mapping as Step 5a. Do not touch issue links + link type and direction mapping as Step 6a. Do not touch issue links of other types. Update only the sync-owned fields. Do not touch assignee or other Jira-managed fields. For **Reopened** items, transition the Jira issue back to an open status before updating content (see the status rule in -Step 4b). Do not change status for any other reason. +Step 5b). Do not change status for any other reason. After updating, record the new hash using `update-manifest --operation update` with `--item-type story` and `--epic-file` from the helper @@ -590,7 +602,7 @@ reference. **If update fails:** Report the error with the Jira key and offer to retry or skip. -#### 5c: Close Removed and Selected Orphaned Stories +#### 6c: Close Removed and Selected Orphaned Stories For each story categorized as **Removed** (file has `status: removed`) or **Orphaned** where the user chose to close it in Step 1: @@ -617,9 +629,9 @@ Present the stories synced for each epic: | Closed | Story 1.03 | {EDM-CCCC} | [{prefix}] {title} | ``` -### Step 6: Verify Sync Manifest +### Step 7: Verify Sync Manifest -The manifest has been built/updated incrementally during Steps 4-5 (each +The manifest has been built/updated incrementally during Steps 5-6 (each operation was recorded via `update-manifest` immediately after success). Verify the manifest is complete and consistent. The final structure should be: @@ -659,16 +671,16 @@ Fields: - `synced_at` — top-level only, not per-entry. Updated to the current timestamp at the end of each sync run. -### Step 7: Report to User +### Step 8: Report to User Summarize: - How many epics and stories were created, updated, closed, and unchanged -- Confirm the hierarchy was verified: every epic has parent = Feature (verified in Step 4), every story has parent = its epic (verified in Step 5) +- Confirm the hierarchy was verified: every epic has parent = Feature (verified in Step 5), every story has parent = its epic (verified in Step 6) - Link to the Feature issue in Jira (which now shows the full hierarchy) **Do not suggest manual parent linking as a next step.** If any parent link was not set during creation, that is a failure that should have been -caught in Steps 4-5. +caught in Steps 5-6. Present a translation table mapping all local artifact identifiers to Jira keys. This table is the definitive reference for anyone working with the From e2c9b935ab7f25477e2bde03bf4afad9f50a225e Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 20:12:08 +0000 Subject: [PATCH 05/12] Fix test step-naming accuracy, reviewer severity scope, reference guard, reopened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1: Correct test_sync_helper.py method names/docstrings/variables to match actual sync sections — test_step5c_* inspect _step5c (epic cascade), test_step6c_* inspect _step6c (story close). Fix class docstring, regex anchors, and assert messages. No duplicate methods. R2: Fix decomposition-review MEDIUM severity to scope marker checks to PRD-observable evidence; decomposer verifies design-derived provenance in Step 13. R4: Replace inaccurate global ordering claim with per-epic processing truth. Add reference-availability guard: before rendering a DOCS update, verify contributor keys exist in manifest; defer if missing. Add 4 contract tests for same-epic, cross-epic, missing-key, and filename-stability scenarios. R5: Explicitly include reopened contributors in respond.md affected set. Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 2 +- design/scripts/test_sync_helper.py | 194 +++++++++++++++++++++++------ design/skills/decompose.md | 7 +- design/skills/respond.md | 4 +- design/skills/sync.md | 24 ++-- 5 files changed, 180 insertions(+), 51 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index 2ac925ed..b954f9fd 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -192,7 +192,7 @@ Reference section. |----------|---------|--------| | CRITICAL | Would cause incorrect implementation — missing requirements, contradictory acceptance criteria, broken dependency chain, scope reduction, stories referencing non-existent ICs or TCs, ICs with no implementing story, prerequisite story without justification or dependency declaration, `[DOCS]` story missing contributing stories or with Documentation Inputs that omit user-facing artifacts from referenced implementation stories | Must fix before presenting | | HIGH | Would cause rework — story too large to review in a single PR, missing testing commitment, integration gap between stories, `[QE]` story duplicating `[DEV]` test scope, stories missing Interface Changes or Validated by references, prerequisite story missing preserved-behavior tests, `[DOCS]` story with generic title or unidentified reader/task | Should fix before presenting | -| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story missing `[Assumption]` or `[Open: §9.N]` markers on Documentation Inputs items that are not derived from the design or PRD (planned items need no marker) | Fix if it adds value | +| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story with Documentation Inputs items that appear speculative or unresolved from PRD context but lack `[Assumption]` or `[Open: §9.N]` markers (items verifiable against the PRD need no marker; the decomposer verifies design-derived provenance separately in Step 13) | Fix if it adds value | | LOW | Structural improvements — naming consistency, ordering suggestions, documentation clarity | Fix only if clearly valuable | ## Validation Rules diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index 9eb3c408..7f10fb08 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -2264,7 +2264,7 @@ class TestOrphanClosureStateful(_TmpDirMixin, unittest.TestCase): The helper never contacts Jira; tests simulate the effect of successful Jira operations by calling update-manifest with the correct arguments. This mirrors the workflow described in - Steps 4c/5c of sync.md. + Steps 5c/6c of sync.md. """ def setUp(self) -> None: super().setUp() @@ -2851,7 +2851,7 @@ def test_metacharacter_paths_no_execution(self) -> None: # --------------------------------------------------------------------------- class TestCascadeGateContract(unittest.TestCase): - """Assert sync.md Steps 4c/5c document hash-before-transition ordering + """Assert sync.md Steps 5c/6c document hash-before-transition ordering and that Step 5c gates the parent close on all child successes. These are contract tests against the ACTUAL Markdown sections/order, @@ -2869,28 +2869,28 @@ def setUpClass(cls) -> None: import re sync_md_path = Path(__file__).resolve().parent.parent / "skills" / "sync.md" cls._sync_md = sync_md_path.read_text(encoding="utf-8") - m4c = re.search(r"#### 5c:.*?(?=####|### )", cls._sync_md, re.DOTALL) - assert m4c, "Step 6c section not found" - cls._step5c = m4c.group() - m5c = re.search(r"#### 6c:.*?(?=####|### )", cls._sync_md, re.DOTALL) - assert m5c, "Step 6c section not found" - cls._step6c = m5c.group() - m5 = re.search(r"### Step 6:.*?(?=#### 6a:)", cls._sync_md, re.DOTALL) - assert m5, "Step 6 plan refresh not found" - cls._step6_refresh = m5.group() + m5c = re.search(r"#### 5c:.*?(?=####|### )", cls._sync_md, re.DOTALL) + assert m5c, "Step 5c section not found" + cls._step5c = m5c.group() + m6c = re.search(r"#### 6c:.*?(?=####|### )", cls._sync_md, re.DOTALL) + assert m6c, "Step 6c section not found" + cls._step6c = m6c.group() + m6 = re.search(r"### Step 6:.*?(?=#### 6a:)", cls._sync_md, re.DOTALL) + assert m6, "Step 6 plan refresh not found" + cls._step6_refresh = m6.group() # -- Step 5c: child hash before child transition --- - def test_step6c_child_hash_before_transition(self) -> None: - """6c child closure: hash computed before Jira transition.""" + def test_step5c_child_hash_before_transition(self) -> None: + """5c child closure: hash computed before Jira transition.""" h = self._step5c.find("Compute the hash first") t = self._step5c.find("Transition the child story") self.assertGreater(h, -1, "must mention computing child hash") self.assertGreater(t, -1, "must mention child transition") self.assertLess(h, t, "child hash must precede child transition") - def test_step6c_epic_hash_before_transition(self) -> None: - """6c epic closure: hash computed before Jira transition.""" + def test_step5c_epic_hash_before_transition(self) -> None: + """5c epic closure: hash computed before Jira transition.""" h = self._step5c.find("compute the epic hash") t = self._step5c.find("Transition the epic to Done") self.assertGreater(h, -1, "must mention computing epic hash") @@ -2899,50 +2899,50 @@ def test_step6c_epic_hash_before_transition(self) -> None: # -- Step 5c: parent gated on all children --- - def test_step6c_parent_gated_on_all_children(self) -> None: - """4c: parent close requires ALL child closures and checkpoints.""" + def test_step5c_parent_gated_on_all_children(self) -> None: + """5c: parent close requires ALL child closures and checkpoints.""" self.assertIn("child closures and their checkpoints have succeeded", self._step5c) - def test_step6c_stop_cascade_on_failure(self) -> None: - """4c: cascade stops immediately on any child failure.""" + def test_step5c_stop_cascade_on_failure(self) -> None: + """5c: cascade stops immediately on any child failure.""" import re self.assertTrue( re.search(r"stop the cascade\s+immediately", self._step5c), "must stop cascade immediately on child failure") - def test_step6c_no_skip_child_then_close_parent(self) -> None: - """4c: explicitly forbids skipping a child then closing parent.""" + def test_step5c_no_skip_child_then_close_parent(self) -> None: + """5c: explicitly forbids skipping a child then closing parent.""" import re self.assertTrue( re.search(r"Never\s+skip\s+a failing child", self._step5c), "must forbid skip-child-close-parent") - def test_step6c_offers_retry_or_defer(self) -> None: - """4c: pre-transition failure offers retry or defer.""" + def test_step5c_offers_retry_or_defer(self) -> None: + """5c: pre-transition failure offers retry or defer.""" import re self.assertTrue( re.search(r"retry after\s+remediation", self._step5c), "must offer retry after remediation") self.assertIn("defer the entire parent", self._step5c) - def test_step6c_preserves_successful_checkpoints(self) -> None: - """4c: already-successful child checkpoints preserved on failure.""" + def test_step5c_preserves_successful_checkpoints(self) -> None: + """5c: already-successful child checkpoints preserved on failure.""" import re self.assertTrue( re.search(r"Preserve all\s+already-successful\s+child checkpoints", self._step5c), "must preserve completed child checkpoints") - def test_step6c_distinguishes_pre_and_post_failure(self) -> None: - """4c: distinguishes pre-transition from post-success checkpoint + def test_step5c_distinguishes_pre_and_post_failure(self) -> None: + """5c: distinguishes pre-transition from post-success checkpoint failure, referencing the shared close-checkpoint recovery rule.""" self.assertIn("Pre-transition failure", self._step5c) self.assertIn("Post-success checkpoint failure", self._step5c) self.assertIn("close-checkpoint", self._step5c) - def test_step6c_uncertain_outcome_requires_verification(self) -> None: - """4c: uncertain Jira outcome (timeout/5xx) must be verified + def test_step5c_uncertain_outcome_requires_verification(self) -> None: + """5c: uncertain Jira outcome (timeout/5xx) must be verified before retrying, not assumed non-mutating.""" import re self.assertTrue( @@ -2950,8 +2950,8 @@ def test_step6c_uncertain_outcome_requires_verification(self) -> None: self._step5c, re.DOTALL), "must require verification for uncertain Jira outcomes") - def test_step6c_resume_manual_reconciliation(self) -> None: - """4c: resume stops for manual reconciliation when captured + def test_step5c_resume_manual_reconciliation(self) -> None: + """5c: resume stops for manual reconciliation when captured state is lost (forced abort/new run).""" self.assertIn("stop for manual reconciliation", self._step5c) @@ -2966,13 +2966,13 @@ def test_step6c_hash_before_transition(self) -> None: self.assertLess(h, t, "hash must precede transition") def test_step6c_checkpoint_uses_saved_hash(self) -> None: - """5c: checkpoint references saved hash from step 1.""" + """6c: checkpoint references saved hash from step 1.""" self.assertIn("hash from step 1", self._step6c) # -- Step 5c: reporting distinguishes child states --- - def test_step6c_report_distinguishes_child_states(self) -> None: - """4c: failure report distinguishes complete, uncheckpointed, + def test_step5c_report_distinguishes_child_states(self) -> None: + """5c: failure report distinguishes complete, uncheckpointed, and unattempted children.""" import re self.assertIn("closed and checkpointed (complete)", self._step5c) @@ -2981,8 +2981,8 @@ def test_step6c_report_distinguishes_child_states(self) -> None: "must mention Jira-closed but uncheckpointed") self.assertIn("unattempted", self._step5c) - def test_step6c_no_rollback_implication(self) -> None: - """4c: does not imply already-closed children are rolled back.""" + def test_step5c_no_rollback_implication(self) -> None: + """5c: does not imply already-closed children are rolled back.""" import re self.assertTrue( re.search(r"not imply that already-closed children\s+will be rolled back", @@ -2991,8 +2991,8 @@ def test_step6c_no_rollback_implication(self) -> None: # -- Step 5c: child checkpoint references helper reference --- - def test_step6c_child_checkpoint_references_helper(self) -> None: - """6c child checkpoint step references the helper reference.""" + def test_step5c_child_checkpoint_references_helper(self) -> None: + """5c child checkpoint step references the helper reference.""" self.assertIn("helper reference", self._step5c) # The step must mention the required story-specific args self.assertIn("--item-type story", self._step5c) @@ -4054,5 +4054,123 @@ def test_story_prepare_no_size_requirement(self) -> None: self.assertIn("description", result) +# --------------------------------------------------------------------------- +# Post-sync DOCS placement: synced DOCS preceding later contributor +# --------------------------------------------------------------------------- + +class TestSyncedDocsPlacement(_TmpDirMixin, unittest.TestCase): + """Verify that a synced DOCS story (immutable filename) can reference + a later same-epic contributor after that contributor is created and + recorded in the manifest. Also verify cross-cutting last-epic case + and the missing-key guard.""" + + def setUp(self) -> None: + super().setUp() + self.stories_dir = Path(self.tmpdir) / "06-stories" + self.stories_dir.mkdir() + # Epic 1: DOCS story-03 precedes contributor story-04 + _make_epic(self.stories_dir, 1, "rollback") + _make_dev_story(self.stories_dir, 1, 1, "rollback-cmd", title="Rollback cmd") + _make_docs_story(self.stories_dir, 1, 3, "rollback-docs", + title="Rollback CLI reference", + deps="Story 1.01, Story 1.04", + doc_inputs="**Story 1.01 — Rollback cmd:** subcommand.\n" + "**Story 1.04 — Dry-run flag:** --dry-run flag.", + contributing_stories="Story 1.01, Story 1.04") + _make_dev_story(self.stories_dir, 1, 4, "dry-run-flag", title="Dry-run flag") + + def test_same_epic_contributor_after_docs_resolves(self) -> None: + """After story-04 is created and recorded, story-03 DOCS refs resolve.""" + manifest = { + "epics": [{ + "file": "epic-1-rollback.md", + "jira_key": "EDM-100", + "stories": [ + {"file": "story-01-rollback-cmd.md", "jira_key": "EDM-101"}, + {"file": "story-03-rollback-docs.md", "jira_key": "EDM-103"}, + {"file": "story-04-dry-run-flag.md", "jira_key": "EDM-104"}, + ] + }] + } + docs_path = self.stories_dir / "epic-1" / "story-03-rollback-docs.md" + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + # Contributing stories resolved — no [unresolved] + self.assertIn("Contributing stories: Story EDM-101, Story EDM-104", desc) + self.assertNotIn("[unresolved]", desc) + # Dependencies also resolved + self.assertIn("EDM-101", desc) + self.assertIn("EDM-104", desc) + + def test_missing_contributor_key_leaves_unresolved(self) -> None: + """If contributor story-04 is not yet in manifest, ref stays unresolved.""" + manifest = { + "epics": [{ + "file": "epic-1-rollback.md", + "jira_key": "EDM-100", + "stories": [ + {"file": "story-01-rollback-cmd.md", "jira_key": "EDM-101"}, + {"file": "story-03-rollback-docs.md", "jira_key": "EDM-103"}, + # story-04 NOT in manifest — creation pending/failed + ] + }] + } + docs_path = self.stories_dir / "epic-1" / "story-03-rollback-docs.md" + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + # Story 1.04 should be unresolved + self.assertIn("[unresolved]", desc) + # Story 1.01 still resolved + self.assertIn("EDM-101", desc) + + def test_cross_epic_contributor_resolves_after_creation(self) -> None: + """Cross-cutting DOCS in epic 1 referencing a contributor in epic 2.""" + # Add epic 2 with a contributor + _make_epic(self.stories_dir, 2, "monitoring") + _make_dev_story(self.stories_dir, 2, 1, "metrics", title="Add metrics") + # Update DOCS story to reference cross-epic contributor + docs_path = self.stories_dir / "epic-1" / "story-03-rollback-docs.md" + content = docs_path.read_text() + content = content.replace( + "Contributing stories: Story 1.01, Story 1.04", + "Contributing stories: Story 1.01, Story 1.04, Story 2.01") + docs_path.write_text(content) + + manifest = { + "epics": [ + { + "file": "epic-1-rollback.md", + "jira_key": "EDM-100", + "stories": [ + {"file": "story-01-rollback-cmd.md", "jira_key": "EDM-101"}, + {"file": "story-03-rollback-docs.md", "jira_key": "EDM-103"}, + {"file": "story-04-dry-run-flag.md", "jira_key": "EDM-104"}, + ] + }, + { + "file": "epic-2-monitoring.md", + "jira_key": "EDM-200", + "stories": [ + {"file": "story-01-metrics.md", "jira_key": "EDM-201"}, + ] + } + ] + } + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + # All three contributor refs resolved + self.assertIn("EDM-101", desc) + self.assertIn("EDM-104", desc) + self.assertIn("EDM-201", desc) + self.assertNotIn("[unresolved]", desc) + + def test_docs_filename_identity_stable(self) -> None: + """DOCS story filename/number is immutable in the manifest.""" + docs_path = self.stories_dir / "epic-1" / "story-03-rollback-docs.md" + self.assertTrue(docs_path.exists()) + # The file is story-03, not story-05 — it was not renumbered + self.assertEqual(docs_path.name, "story-03-rollback-docs.md") + + if __name__ == "__main__": unittest.main() diff --git a/design/skills/decompose.md b/design/skills/decompose.md index c3c68341..1bbe4cf4 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -365,9 +365,10 @@ validation gate. exemption:** once a `[DOCS]` story has been synced (its filename is in the manifest), it retains its immutable filename and number even when a later contributor is appended with a higher story number. - This is safe because `/sync` creates new stories before updating - existing ones, so the contributor's Jira key is in the manifest - when the `[DOCS]` story's description is rendered. + `/sync` verifies that every referenced contributor's Jira key is + available in the manifest before rendering the `[DOCS]` story's + description; if a key is missing, the update is deferred until the + contributor is created. **Story quality checks:** - For `[DEV]` stories: does this story add a capability AND testing for that capability? For a justified behavior-preserving prerequisite, does it restructure existing code AND include tests demonstrating preserved behavior? diff --git a/design/skills/respond.md b/design/skills/respond.md index cb58b72e..fbe03ddc 100644 --- a/design/skills/respond.md +++ b/design/skills/respond.md @@ -525,8 +525,8 @@ re-running `/decompose`. **`[DOCS]` story re-evaluation:** If decomposition artifacts exist (`06-stories/`), re-evaluate every affected `[DOCS]` story against all seven criteria in `decompose.md`. A `[DOCS]` story is affected -when this round changed: (a) the `[DOCS]` file itself, (b) a listed -or newly relevant `[DEV]`/`[UI]` contributor's artifacts, (c) design +when this round changed: (a) the `[DOCS]` file itself, (b) a listed, +newly relevant, or reopened `[DEV]`/`[UI]` contributor's artifacts, (c) design content that alters the story's reader, scope, AC, assumptions, or open-question markers — including resolved `[Open: §9.N]` items even without API/CLI shape changes, or (d) ICs covered by the story. diff --git a/design/skills/sync.md b/design/skills/sync.md index 3dce5e1a..99917e1d 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -262,13 +262,23 @@ reopened), not only from existing references. When evaluating criterion 7 (placement), synced `[DOCS]` stories retain their immutable filename and number. If a contributor is appended with a higher story number after the `[DOCS]` story was -synced, the `[DOCS]` story may numerically precede it. This does not -fail the gate because sync processes stories in numeric order within -each epic: new contributors are created in Step 6a before existing -`[DOCS]` stories are updated in Step 6b, so the contributor's Jira -key is available in the manifest at `[DOCS]` rendering time. The -same applies to cross-cutting `[DOCS]` stories whose last-epic -contributors are appended after the `[DOCS]` story number. +synced, the `[DOCS]` story may numerically precede it — do not fail +this gate solely for that ordering. + +**Reference-availability guard for `[DOCS]` updates.** Step 6 +processes stories per epic in three passes (create, update, close). +Within the same epic, a new contributor created in Step 6a will have +its Jira key in the manifest before an existing `[DOCS]` story is +updated in Step 6b. For cross-cutting `[DOCS]` stories or +contributors in a later epic, the contributor's Jira key may not yet +be available when the `[DOCS]` story's epic is processed. Before +rendering a `[DOCS]` story's description in Step 6b, verify that +every story referenced in its Contributing stories and Dependencies +has a Jira key in the manifest. If any required key is missing +(contributor creation was skipped, failed, or is in a not-yet- +processed epic), defer that `[DOCS]` update — do not render with +`[unresolved]` references. Process it after the missing contributor's +epic completes, or report the gap and recommend retry. If any `[DOCS]` story fails validation, present the findings and recommend `/revise` — sync must not modify story source files. Do From f48a783cd2a54c708dde8829f23232c3dd7f7630 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 20:36:25 +0000 Subject: [PATCH 06/12] Consolidate decompose to 10 steps; extend sync guard to creation; fix respond/review scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A: decomposition-review criterion 8 now flags items that appear speculative OR unresolved from PRD context, not only speculative. Provenance pointer uses semantic reference to decompose Step 8. B: respond.md affected-set explicitly includes removed and renamed contributors alongside listed/newly-relevant/reopened ones. C: sync reference-availability guard now protects DOCS creation (6a), updates (6b), and reopening (6b) — not only updates. Deferred DOCS stories skip Jira write AND checkpoint, remaining eligible for retry. D: Consolidate decompose.md from 15 to 10 main steps: old 1+2→1 (load source/feature), 3→2 (plan), 4→3 (epics), 5+6+7→4 (stories/sizing/references), 8→5 (coverage), 9→6 (structure), 10→7 (review), 11+12+13→8 (validate/re-review/provenance), 14→9 (summary), 15→10 (present). All actions, gates, ordering, reviewer independence, 2-round cap, provenance check, and FLAG behavior preserved. Update external refs in revise.md and decomposition-review.md to semantic pointers. Add 7 contract tests for phase count/order and DOCS creation defer/retry scenarios. Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 7 +- design/scripts/test_sync_helper.py | 116 +++++++++++++++++++++++++++++ design/skills/decompose.md | 60 ++++++++------- design/skills/respond.md | 3 +- design/skills/revise.md | 2 +- design/skills/sync.md | 30 ++++---- 6 files changed, 169 insertions(+), 49 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index b954f9fd..01e18402 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -162,8 +162,9 @@ impact: complete? Note: the reviewer cannot verify whether unmarked Documentation Inputs items are truly derived from the design document or PRD — that provenance check belongs to the decomposer (see - `/decompose` Step 13). Flag only items that appear speculative from - the PRD and artifact context alone. + the provenance verification in `/decompose`). Flag items that + appear speculative or unresolved from the PRD and artifact context + alone. ## Finding Format @@ -192,7 +193,7 @@ Reference section. |----------|---------|--------| | CRITICAL | Would cause incorrect implementation — missing requirements, contradictory acceptance criteria, broken dependency chain, scope reduction, stories referencing non-existent ICs or TCs, ICs with no implementing story, prerequisite story without justification or dependency declaration, `[DOCS]` story missing contributing stories or with Documentation Inputs that omit user-facing artifacts from referenced implementation stories | Must fix before presenting | | HIGH | Would cause rework — story too large to review in a single PR, missing testing commitment, integration gap between stories, `[QE]` story duplicating `[DEV]` test scope, stories missing Interface Changes or Validated by references, prerequisite story missing preserved-behavior tests, `[DOCS]` story with generic title or unidentified reader/task | Should fix before presenting | -| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story with Documentation Inputs items that appear speculative or unresolved from PRD context but lack `[Assumption]` or `[Open: §9.N]` markers (items verifiable against the PRD need no marker; the decomposer verifies design-derived provenance separately in Step 13) | Fix if it adds value | +| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story with Documentation Inputs items that appear speculative or unresolved from PRD context but lack `[Assumption]` or `[Open: §9.N]` markers (items verifiable against the PRD need no marker; the decomposer verifies design-derived provenance separately in the provenance sub-step of Step 8) | Fix if it adds value | | LOW | Structural improvements — naming consistency, ordering suggestions, documentation clarity | Fix only if clearly valuable | ## Validation Rules diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index 7f10fb08..c4745751 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4172,5 +4172,121 @@ def test_docs_filename_identity_stable(self) -> None: self.assertEqual(docs_path.name, "story-03-rollback-docs.md") +# --------------------------------------------------------------------------- +# Phase-count and ordering contract for decompose.md +# --------------------------------------------------------------------------- + +class TestDecomposePhaseContract(unittest.TestCase): + """Verify decompose.md has <=10 sequential main steps and preserves + key ordering dependencies.""" + + _decompose_md: str + + @classmethod + def setUpClass(cls) -> None: + import re + path = Path(__file__).resolve().parent.parent / "skills" / "decompose.md" + cls._decompose_md = path.read_text(encoding="utf-8") + cls._steps = re.findall(r"^### Step (\d+):", cls._decompose_md, re.MULTILINE) + + def test_at_most_10_main_steps(self) -> None: + """decompose.md must have <= 10 main steps.""" + self.assertLessEqual(len(self._steps), 10, + f"Expected <= 10 main steps, found {len(self._steps)}") + + def test_steps_sequential(self) -> None: + """Main steps are numbered 1..N sequentially.""" + nums = [int(s) for s in self._steps] + self.assertEqual(nums, list(range(1, len(nums) + 1)), + f"Steps not sequential: {nums}") + + def test_review_after_structure_gate(self) -> None: + """Independent review (Step 7) comes after artifact structure check.""" + review_idx = self._decompose_md.find("### Step 7: Review") + structure_idx = self._decompose_md.find("### Step 6: Verify Artifact") + self.assertGreater(review_idx, structure_idx, + "Review must follow structure verification") + + def test_provenance_after_review(self) -> None: + """Provenance verification is inside Step 8, after independent review.""" + provenance_idx = self._decompose_md.find("Verify `[DOCS]` input provenance") + review_idx = self._decompose_md.find("### Step 7: Review") + self.assertGreater(provenance_idx, review_idx, + "Provenance must come after independent review") + + def test_present_is_last_step(self) -> None: + """User presentation is the final step.""" + self.assertEqual(self._steps[-1], "10") + self.assertIn("Present to User", self._decompose_md.split("### Step 10:")[1][:50]) + + +# --------------------------------------------------------------------------- +# Sync: deferred DOCS creation with missing contributor key +# --------------------------------------------------------------------------- + +class TestSyncDocsCreationDefer(_TmpDirMixin, unittest.TestCase): + """A new DOCS story whose contributor has no manifest key should + produce [unresolved] markers in render output, signaling that + the sync agent must defer creation and not checkpoint.""" + + def test_new_docs_missing_contributor_no_checkpoint(self) -> None: + """New DOCS story with missing contributor key renders [unresolved].""" + stories_dir = Path(self.tmpdir) / "06-stories" + stories_dir.mkdir() + _make_epic(stories_dir, 1, "feature") + _make_dev_story(stories_dir, 1, 1, "api", title="API endpoint") + _make_docs_story(stories_dir, 1, 2, "docs", + title="API reference", + deps="Story 1.01, Story 1.03", + contributing_stories="Story 1.01, Story 1.03", + doc_inputs="**Story 1.01:** endpoint.\n**Story 1.03:** webhook.") + # story-03 exists on disk but NOT in manifest + _make_dev_story(stories_dir, 1, 3, "webhook", title="Webhook") + + manifest = {"epics": [{ + "file": "epic-1-feature.md", + "jira_key": "T-100", + "stories": [ + {"file": "story-01-api.md", "jira_key": "T-101"}, + # story-02 (DOCS) and story-03 (DEV) not yet created + ] + }]} + + docs_path = stories_dir / "epic-1" / "story-02-docs.md" + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + # Story 1.03 has no key → must show [unresolved] + self.assertIn("[unresolved]", desc, + "Missing contributor key must produce [unresolved]") + # Story 1.01 should still resolve + self.assertIn("T-101", desc) + + def test_new_docs_all_keys_available_resolves(self) -> None: + """New DOCS story with all contributor keys resolves cleanly.""" + stories_dir = Path(self.tmpdir) / "06-stories" + stories_dir.mkdir() + _make_epic(stories_dir, 1, "feature") + _make_dev_story(stories_dir, 1, 1, "api", title="API endpoint") + _make_docs_story(stories_dir, 1, 2, "docs", + title="API reference", + deps="Story 1.01", + contributing_stories="Story 1.01", + doc_inputs="**Story 1.01:** endpoint.") + + manifest = {"epics": [{ + "file": "epic-1-feature.md", + "jira_key": "T-100", + "stories": [ + {"file": "story-01-api.md", "jira_key": "T-101"}, + ] + }]} + + docs_path = stories_dir / "epic-1" / "story-02-docs.md" + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + self.assertNotIn("[unresolved]", desc) + self.assertIn("T-101", desc) + + if __name__ == "__main__": unittest.main() diff --git a/design/skills/decompose.md b/design/skills/decompose.md index 1bbe4cf4..ceeca3ee 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -29,7 +29,7 @@ must leave the system in a stable state. ## Process -### Step 1: Read Source Material +### Step 1: Load Source Material and Identify Feature Read these files: 1. `.artifacts/design/{issue-key}/03-design.md` (design document) @@ -44,12 +44,10 @@ run first. If the testplan doesn't exist, tell the user that `/draft` should be run first. -### Step 2: Identify the Feature - Read the Jira Feature issue key from the PRD or context. This Feature is the parent for all epics created by this workflow. -### Step 3: Plan the Decomposition +### Step 2: Plan the Decomposition Before writing, plan: @@ -86,7 +84,7 @@ Before writing, plan: split into smaller epics before proceeding. Split by user-value slice, not technical layer. -### Step 4: Write Epics +### Step 3: Write Epics This step produces two things: a metadata index (`05-epics.md`) and one file per epic. Each epic file has a 1:1 correspondence with a Jira Epic issue — @@ -161,7 +159,7 @@ Design sections: {§4.1 Architecture, §4.3 API Changes, or relevant subsections - Gaps in epic numbering are acceptable (e.g., if epic-2 is removed during revision, epic-1 and epic-3 remain — do not renumber to close the gap) -### Step 5: Write Stories +### Step 4: Write Stories, Verify Sizing, and Add References For each epic, create a story directory and write one file per story. Each story file has a 1:1 correspondence with a Jira Story issue — its content @@ -223,7 +221,7 @@ Validated by: {TC-FR1-01, TC-FR3-02} `Interface Changes` lists the ICs from the design's §5 that this story implements. `Validated by` lists the TCs from the testplan that validate the behavior this story delivers. These references are populated in -Step 7 after stories are written. +the implementation-references sub-step of Step 4 after stories are written. **For `[DOCS]` stories** (see `[DOCS]` story quality criteria below): @@ -404,7 +402,7 @@ to Jira, their filenames must not change. When revising a decomposition, append new stories with the next available number rather than inserting or renumbering existing ones. -### Step 6: Sizing Consistency Check +#### Sizing Consistency Check After sizing all epics, verify plausibility: @@ -421,14 +419,14 @@ After sizing all epics, verify plausibility: epics. Flag any mismatch to the user. 3. Verify no epic is sized XXL. If any is, stop and require a split - before proceeding to Step 7. + before proceeding to the implementation references sub-step. -### Step 7: Add Implementation References to Stories +#### Add Implementation References to Stories -For each non-`[DOCS]` story written in Step 5, populate the `Interface +For each non-`[DOCS]` story written above, populate the `Interface Changes` and `Validated by` fields in the Design Reference section. -#### 7a: Map Stories to Interface Changes +**Map stories to Interface Changes.** For each non-`[DOCS]` story, determine which Interface Changes (IC-N) from the design's §5 the story implements. Use the story's Design @@ -439,7 +437,7 @@ A story implements an IC when it delivers part or all of the behavior that IC describes. One story may implement multiple ICs, and one IC may be implemented across multiple stories. -#### 7b: Map Stories to Test Cases +**Map stories to test cases.** For each non-`[DOCS]` story, determine which test cases from the testplan (read in Step 1) validate the behavior the story delivers. @@ -458,7 +456,7 @@ For stories that implement ICs with no corresponding test cases story's `Validated by` line should read: `Validated by: None — {reason from testplan gaps}` -#### 7c: Write References +**Write references.** Re-read each non-`[DOCS]` story file. Update the `Interface Changes` and `Validated by` lines in the Design Reference section: @@ -483,7 +481,7 @@ Interface Changes: None — behavior-preserving prerequisite Validated by: None — behavior-preserving prerequisite; existing tests verify preserved behavior ``` -### Step 8: Write Coverage Matrix +### Step 5: Write Coverage Matrix Write `.artifacts/design/{issue-key}/07-coverage.md`: @@ -512,7 +510,7 @@ Write `.artifacts/design/{issue-key}/07-coverage.md`: If none: "All stories trace to PRD requirements."} ``` -### Step 9: Verify Artifact Structure +### Step 6: Verify Artifact Structure Quick sanity check before invoking the decomposition review. Verify: @@ -521,13 +519,13 @@ Quick sanity check before invoking the decomposition review. Verify: `06-stories/epic-1-{slug}.md`) 3. Each epic has a corresponding story directory with story files 4. `07-coverage.md` exists and contains at least one mapping row -5. Testplan was loaded in Step 1 (confirmed — Step 1 blocks if missing) +5. Testplan was loaded in Step 1 (confirmed — source loading blocks if missing) 6. Every non-`[DOCS]` story file has `Interface Changes` and `Validated by` lines in its Design Reference section If structural issues are found, fix them before proceeding. Do not invoke a review on incomplete artifacts. -### Step 10: Review Decomposition +### Step 7: Review Decomposition Review the decomposition for structural quality and requirement coverage. This review operates independently from the design document — @@ -551,7 +549,7 @@ subagent for independence. Load it with: - NOT the design document (`03-design.md`) — the reviewer evaluates the artifacts on their own merits -Retain the subagent's ID for use in Step 12 — resuming the same +Retain the subagent's ID for use in the re-review sub-step — resuming the same reviewer gives it memory of its previous findings and concerns, producing more coherent follow-up reviews. @@ -566,9 +564,9 @@ severity definitions from the protocol. The subagent path provides stronger independence; the inline path still catches issues by forcing a perspective shift. -### Step 11: Validate and Assess Findings +### Step 8: Validate Findings, Re-Review, and Verify Provenance -For each finding from Step 10: +For each finding from Step 7: 1. **Validate the reference.** Confirm the cited artifact file and section exist. Discard any finding that references a file or section @@ -591,13 +589,13 @@ For each finding from Step 10: Only fix findings that add real value. Do not make changes for structural preferences not grounded in the evaluation criteria. -### Step 12: Re-Review (if fixes were made) +#### Re-review (if fixes were made) -If Step 11 produced changes to the decomposition artifacts: +If the validation above produced changes to the decomposition artifacts: 1. Obtain a re-review of the updated artifacts: - **If a subagent was used in Step 10 and the runtime supports agent + **If a subagent was used in Step 7 and the runtime supports agent resumption:** Resume the same reviewer agent. Send it the updated artifacts and a summary of fixes applied. This gives the reviewer memory of its original findings and lets it verify they were @@ -617,13 +615,13 @@ If Step 11 produced changes to the decomposition artifacts: - Whether fixes were applied correctly - Whether fixes introduced new issues 2. If new issues are found, fix them following the same validate-and- - assess procedure from Step 11 + assess procedure above 3. Cap at 2 review-fix rounds total. Decomposition fixes are structural and less likely than code fixes to need multiple iterations. -If no fixes were needed in Step 11, the review passes immediately. +If no fixes were needed in the validation above, the review passes immediately. -### Step 13: Verify `[DOCS]` Input Provenance +#### Verify `[DOCS]` input provenance The independent reviewer cannot verify whether unmarked Documentation Inputs items are truly derived from the design document or PRD — it @@ -639,9 +637,9 @@ depends on an unresolved open question must be marked `[Open: §9.N]` — use the existing open-question identifier from the design document; do not invent new identifiers for this purpose. -Fix any incorrect markers before proceeding to Step 14. +Fix any incorrect markers before proceeding to Step 9. -### Step 14: Report Review Summary +### Step 9: Report Review Summary ```markdown ## Decomposition Review Summary @@ -656,7 +654,7 @@ Fix any incorrect markers before proceeding to Step 14. list them with their file, section, and issue description.} ``` -### Step 15: Present to User +### Step 10: Present to User Present the decomposition and highlight: - Number of epics and stories @@ -664,7 +662,7 @@ Present the decomposition and highlight: - Any coverage gaps - Stories that might need size adjustment (too large or too small) - Any assumptions or judgment calls in the decomposition -- Decomposition review summary (from Step 14) +- Decomposition review summary (from Step 9) If the decomposition review gate reported FLAG, present the unfixed CRITICAL/HIGH findings and ask the user to decide how to handle them. diff --git a/design/skills/respond.md b/design/skills/respond.md index fbe03ddc..a1ba3f9d 100644 --- a/design/skills/respond.md +++ b/design/skills/respond.md @@ -526,7 +526,8 @@ re-running `/decompose`. (`06-stories/`), re-evaluate every affected `[DOCS]` story against all seven criteria in `decompose.md`. A `[DOCS]` story is affected when this round changed: (a) the `[DOCS]` file itself, (b) a listed, -newly relevant, or reopened `[DEV]`/`[UI]` contributor's artifacts, (c) design +newly relevant, removed, renamed, or reopened `[DEV]`/`[UI]` +contributor's artifacts, (c) design content that alters the story's reader, scope, AC, assumptions, or open-question markers — including resolved `[Open: §9.N]` items even without API/CLI shape changes, or (d) ICs covered by the story. diff --git a/design/skills/revise.md b/design/skills/revise.md index 8806e944..3f7b1381 100644 --- a/design/skills/revise.md +++ b/design/skills/revise.md @@ -207,7 +207,7 @@ updates above are complete on their own. - **Renamed IC:** Replace the old IC identifier with the new one in every story that lists it. - **Added IC:** Determine which stories implement the new IC using - the same criteria as `/decompose` Step 7a (the story delivers part + the same criteria as `/decompose`'s IC-mapping sub-step (the story delivers part or all of the behavior the IC describes). If ownership is unclear, ask the user which story should claim it. - If test cases were added, removed, or had their IC or requirement diff --git a/design/skills/sync.md b/design/skills/sync.md index 99917e1d..ba79c0c1 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -265,20 +265,24 @@ appended with a higher story number after the `[DOCS]` story was synced, the `[DOCS]` story may numerically precede it — do not fail this gate solely for that ordering. -**Reference-availability guard for `[DOCS]` updates.** Step 6 +**Reference-availability guard for `[DOCS]` stories.** Step 6 processes stories per epic in three passes (create, update, close). -Within the same epic, a new contributor created in Step 6a will have -its Jira key in the manifest before an existing `[DOCS]` story is -updated in Step 6b. For cross-cutting `[DOCS]` stories or -contributors in a later epic, the contributor's Jira key may not yet -be available when the `[DOCS]` story's epic is processed. Before -rendering a `[DOCS]` story's description in Step 6b, verify that -every story referenced in its Contributing stories and Dependencies -has a Jira key in the manifest. If any required key is missing -(contributor creation was skipped, failed, or is in a not-yet- -processed epic), defer that `[DOCS]` update — do not render with -`[unresolved]` references. Process it after the missing contributor's -epic completes, or report the gap and recommend retry. +Within the same epic, a new non-`[DOCS]` contributor created earlier +in Step 6a will have its Jira key in the manifest before a `[DOCS]` +story in the same pass or in Step 6b. For contributors in a later +epic, the key may not yet be available when the `[DOCS]` story's +epic is processed. + +Before rendering any `[DOCS]` story's description — whether for +creation (Step 6a), update (Step 6b), or reopening (Step 6b) — +verify that every story referenced in its Contributing stories and +Dependencies has a Jira key in the manifest. If any required key is +missing (contributor creation was skipped, failed, or is in a +not-yet-processed epic), **defer** that `[DOCS]` write: do not +render, create, or update the Jira issue, and do not record a +manifest checkpoint. The deferred `[DOCS]` story remains eligible +for retry after the missing contributor's epic completes or on a +subsequent `/sync` run. Report deferred stories to the user. If any `[DOCS]` story fails validation, present the findings and recommend `/revise` — sync must not modify story source files. Do From 5fcf627489a3504e8627accdcab83fcf2ee5ec4b Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 20:45:23 +0000 Subject: [PATCH 07/12] Fix test accuracy: existence assertions, valid ordering, helper API, lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decompose phase contract: add existence assertions to all ordering checks so deleting a gate cannot accidentally pass; verify source blocking, feature identification, sizing XXL stop, IC/TC mapping, structure gate, review independence, finding validation, max-2 rounds, provenance even without fixes, FLAG gate, presentation, and [Open: §9.N] preservation. Sync DOCS reference guard: use valid story ordering (contributors before DOCS, story-03 not story-02), model skipped contributor creation, fix detect_changes API (returns dict not JSON string), fix compute_file_hash name, normalize line-wrapped guard text. Add lifecycle tests: deferred DOCS stays NEW on retry, resolves after contributor creation, checkpointed DOCS becomes unchanged. Add instruction contract test for guard coverage of 6a/6b/reopen. Co-Authored-By: Claude Opus 4.6 --- design/scripts/test_sync_helper.py | 308 +++++++++++++++++++++++------ 1 file changed, 247 insertions(+), 61 deletions(-) diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index c4745751..5b9676d9 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4178,7 +4178,7 @@ def test_docs_filename_identity_stable(self) -> None: class TestDecomposePhaseContract(unittest.TestCase): """Verify decompose.md has <=10 sequential main steps and preserves - key ordering dependencies.""" + all original actions, gates, and ordering dependencies.""" _decompose_md: str @@ -4190,102 +4190,288 @@ def setUpClass(cls) -> None: cls._steps = re.findall(r"^### Step (\d+):", cls._decompose_md, re.MULTILINE) def test_at_most_10_main_steps(self) -> None: - """decompose.md must have <= 10 main steps.""" self.assertLessEqual(len(self._steps), 10, f"Expected <= 10 main steps, found {len(self._steps)}") def test_steps_sequential(self) -> None: - """Main steps are numbered 1..N sequentially.""" nums = [int(s) for s in self._steps] self.assertEqual(nums, list(range(1, len(nums) + 1)), f"Steps not sequential: {nums}") + def _assert_marker_exists(self, marker: str, msg: str) -> int: + idx = self._decompose_md.find(marker) + self.assertGreaterEqual(idx, 0, msg) + return idx + + def test_source_loading_blocks_on_missing_design(self) -> None: + self._assert_marker_exists( + "tell the user that `/draft` should be", + "Source loading must block if design/testplan missing") + + def test_feature_identification_in_step1(self) -> None: + step1 = self._decompose_md.split("### Step 2:")[0] + self.assertIn("Feature issue key", step1, + "Step 1 must identify the Feature") + + def test_sizing_xxl_stop(self) -> None: + self._assert_marker_exists( + "stop and require a split", + "Must stop on XXL epics") + + def test_ic_tc_mapping_present(self) -> None: + self._assert_marker_exists( + "Map stories to Interface Changes", + "IC mapping must be present") + self._assert_marker_exists( + "Map stories to test cases", + "TC mapping must be present") + + def test_structure_gate_exists(self) -> None: + idx = self._assert_marker_exists( + "### Step 6: Verify Artifact Structure", + "Structure gate must exist") + self.assertIn("05-epics.md", self._decompose_md[idx:idx+500]) + def test_review_after_structure_gate(self) -> None: - """Independent review (Step 7) comes after artifact structure check.""" - review_idx = self._decompose_md.find("### Step 7: Review") - structure_idx = self._decompose_md.find("### Step 6: Verify Artifact") - self.assertGreater(review_idx, structure_idx, - "Review must follow structure verification") + structure_idx = self._assert_marker_exists( + "### Step 6: Verify Artifact", + "Structure gate must exist for ordering check") + review_idx = self._assert_marker_exists( + "### Step 7: Review", + "Review step must exist for ordering check") + self.assertGreater(review_idx, structure_idx) + + def test_review_independence(self) -> None: + review_section = self._decompose_md.split("### Step 7:")[1].split("### Step 8:")[0] + self.assertIn("NOT the design document", review_section, + "Review must not receive the design document") + + def test_finding_validation_and_value_filtering(self) -> None: + step8 = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + self.assertIn("Validate the reference", step8) + self.assertIn("Assess on value", step8) + + def test_max_two_review_rounds(self) -> None: + self._assert_marker_exists( + "Cap at 2 review-fix rounds", + "Must cap at 2 review-fix rounds") def test_provenance_after_review(self) -> None: - """Provenance verification is inside Step 8, after independent review.""" - provenance_idx = self._decompose_md.find("Verify `[DOCS]` input provenance") - review_idx = self._decompose_md.find("### Step 7: Review") - self.assertGreater(provenance_idx, review_idx, - "Provenance must come after independent review") + review_idx = self._assert_marker_exists( + "### Step 7: Review", + "Review step must exist") + provenance_idx = self._assert_marker_exists( + "Verify `[DOCS]` input provenance", + "Provenance verification must exist") + self.assertGreater(provenance_idx, review_idx) + + def test_provenance_runs_even_without_fixes(self) -> None: + """Provenance check runs after review passes, even if no fixes were needed.""" + step8 = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + # The re-review sub-step says "the review passes immediately" when no fixes + # but provenance verification follows unconditionally after it + pass_idx = step8.find("review passes immediately") + prov_idx = step8.find("Verify `[DOCS]` input provenance") + self.assertGreaterEqual(pass_idx, 0, "No-fix pass text must exist") + self.assertGreaterEqual(prov_idx, 0, "Provenance text must exist") + self.assertGreater(prov_idx, pass_idx, + "Provenance must follow even the no-fix pass path") + + def test_flag_gate_for_critical_high(self) -> None: + self._assert_marker_exists( + "**Gate:** PASS | FLAG", + "FLAG gate must exist") + self._assert_marker_exists( + "CRITICAL or HIGH findings remain unfixed", + "FLAG criteria must reference CRITICAL/HIGH") def test_present_is_last_step(self) -> None: - """User presentation is the final step.""" self.assertEqual(self._steps[-1], "10") - self.assertIn("Present to User", self._decompose_md.split("### Step 10:")[1][:50]) + last = self._decompose_md.split("### Step 10:")[1][:80] + self.assertIn("Present to User", last) + + def test_open_question_design_references_preserved(self) -> None: + """[Open: §9.N] marker format references design document sections, + not decompose step numbers.""" + self.assertIn("[Open: §9.N]", self._decompose_md) # --------------------------------------------------------------------------- -# Sync: deferred DOCS creation with missing contributor key +# Sync: reference-availability guard for DOCS creation and update # --------------------------------------------------------------------------- -class TestSyncDocsCreationDefer(_TmpDirMixin, unittest.TestCase): - """A new DOCS story whose contributor has no manifest key should - produce [unresolved] markers in render output, signaling that - the sync agent must defer creation and not checkpoint.""" - - def test_new_docs_missing_contributor_no_checkpoint(self) -> None: - """New DOCS story with missing contributor key renders [unresolved].""" - stories_dir = Path(self.tmpdir) / "06-stories" - stories_dir.mkdir() - _make_epic(stories_dir, 1, "feature") - _make_dev_story(stories_dir, 1, 1, "api", title="API endpoint") - _make_docs_story(stories_dir, 1, 2, "docs", - title="API reference", - deps="Story 1.01, Story 1.03", - contributing_stories="Story 1.01, Story 1.03", - doc_inputs="**Story 1.01:** endpoint.\n**Story 1.03:** webhook.") - # story-03 exists on disk but NOT in manifest - _make_dev_story(stories_dir, 1, 3, "webhook", title="Webhook") +class TestSyncDocsReferenceGuard(_TmpDirMixin, unittest.TestCase): + """Verify the reference-availability guard: render_story_description + produces [unresolved] when a contributor key is missing, enabling the + sync agent to detect and defer the DOCS write. Tests model valid + story ordering (contributors before DOCS) and simulate a contributor + whose creation was skipped/failed.""" - manifest = {"epics": [{ + def setUp(self) -> None: + super().setUp() + self.stories_dir = Path(self.tmpdir) / "06-stories" + self.stories_dir.mkdir() + self.artifacts_dir = Path(self.tmpdir) + _make_epic(self.stories_dir, 1, "feature") + # story-01 and story-02 are DEV contributors; story-03 is DOCS + _make_dev_story(self.stories_dir, 1, 1, "api", title="API endpoint") + _make_dev_story(self.stories_dir, 1, 2, "webhook", title="Webhook") + _make_docs_story(self.stories_dir, 1, 3, "docs", + title="API reference", + deps="Story 1.01, Story 1.02", + contributing_stories="Story 1.01, Story 1.02", + doc_inputs="**Story 1.01:** endpoint.\n" + "**Story 1.02:** webhook.") + + def _manifest_with_keys(self, story_keys): + """Build a manifest with the given story entries.""" + return {"epics": [{ "file": "epic-1-feature.md", "jira_key": "T-100", - "stories": [ - {"file": "story-01-api.md", "jira_key": "T-101"}, - # story-02 (DOCS) and story-03 (DEV) not yet created - ] + "stories": story_keys, }]} - docs_path = stories_dir / "epic-1" / "story-02-docs.md" + # -- Guard for new DOCS creation (Step 6a) -- + + def test_new_docs_missing_contributor_shows_unresolved(self) -> None: + """New DOCS: contributor story-02 creation skipped → [unresolved]. + The sync agent must not create this DOCS story or checkpoint it.""" + manifest = self._manifest_with_keys([ + {"file": "story-01-api.md", "jira_key": "T-101"}, + # story-02 creation failed/skipped — no key + ]) + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" desc = sync_helper.render_story_description( docs_path, manifest, story_type="DOCS") - # Story 1.03 has no key → must show [unresolved] self.assertIn("[unresolved]", desc, "Missing contributor key must produce [unresolved]") - # Story 1.01 should still resolve + self.assertIn("T-101", desc, + "Available contributor must still resolve") + + def test_new_docs_all_keys_resolves(self) -> None: + """New DOCS: all contributors created → clean resolution, safe to create.""" + manifest = self._manifest_with_keys([ + {"file": "story-01-api.md", "jira_key": "T-101"}, + {"file": "story-02-webhook.md", "jira_key": "T-102"}, + ]) + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + self.assertNotIn("[unresolved]", desc) self.assertIn("T-101", desc) + self.assertIn("T-102", desc) + + # -- Guard for existing DOCS update (Step 6b) -- + + def test_existing_docs_update_missing_new_contributor(self) -> None: + """Update: a new contributor was added but its creation skipped.""" + # DOCS already synced, now references a new story-02 + manifest = self._manifest_with_keys([ + {"file": "story-01-api.md", "jira_key": "T-101"}, + {"file": "story-03-docs.md", "jira_key": "T-103", + "content_hash": "old", "synced_status": "active"}, + # story-02 creation failed — no key + ]) + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + self.assertIn("[unresolved]", desc, + "Update with missing contributor must show [unresolved]") - def test_new_docs_all_keys_available_resolves(self) -> None: - """New DOCS story with all contributor keys resolves cleanly.""" - stories_dir = Path(self.tmpdir) / "06-stories" - stories_dir.mkdir() - _make_epic(stories_dir, 1, "feature") - _make_dev_story(stories_dir, 1, 1, "api", title="API endpoint") - _make_docs_story(stories_dir, 1, 2, "docs", - title="API reference", - deps="Story 1.01", - contributing_stories="Story 1.01", - doc_inputs="**Story 1.01:** endpoint.") + # -- Retry lifecycle: deferred DOCS stays NEW in next detect-changes -- - manifest = {"epics": [{ + def test_deferred_docs_remains_new_for_retry(self) -> None: + """A DOCS story deferred (not checkpointed) remains NEW on next run.""" + # First sync: story-02 skipped, so DOCS was deferred (no checkpoint) + manifest_path = _make_manifest(self.artifacts_dir, [{ "file": "epic-1-feature.md", "jira_key": "T-100", + "title": "Feature", + "size": "M", + "content_hash": "abc", + "synced_status": "active", "stories": [ - {"file": "story-01-api.md", "jira_key": "T-101"}, - ] - }]} - - docs_path = stories_dir / "epic-1" / "story-02-docs.md" + {"file": "story-01-api.md", "jira_key": "T-101", + "title": "[DEV] API endpoint", "content_hash": "def", + "synced_status": "active"}, + # story-02 and story-03 (DOCS) NOT in manifest = deferred + ], + }]) + plan = sync_helper.detect_changes(str(self.artifacts_dir)) + docs_items = [i for i in plan["items"] + if i["file"].endswith("story-03-docs.md")] + self.assertEqual(len(docs_items), 1) + self.assertEqual(docs_items[0]["category"], "new", + "Deferred DOCS (no checkpoint) must be NEW on next run") + + def test_retry_after_contributor_created_resolves(self) -> None: + """After the missing contributor is created and checkpointed, + the DOCS story's references resolve on the retry run.""" + # Simulate: both contributors now in manifest (retry scenario) + manifest = self._manifest_with_keys([ + {"file": "story-01-api.md", "jira_key": "T-101"}, + {"file": "story-02-webhook.md", "jira_key": "T-102"}, + # DOCS still not in manifest — it is being created now + ]) + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" desc = sync_helper.render_story_description( docs_path, manifest, story_type="DOCS") - self.assertNotIn("[unresolved]", desc) - self.assertIn("T-101", desc) + self.assertNotIn("[unresolved]", desc, + "After contributor creation, refs must resolve") + self.assertIn("T-102", desc) + + def test_successful_checkpoint_makes_docs_unchanged(self) -> None: + """After DOCS creation succeeds and is checkpointed, the next + detect-changes run reports it as unchanged.""" + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" + content_hash = sync_helper.compute_file_hash(str(docs_path)) + manifest_path = _make_manifest(self.artifacts_dir, [{ + "file": "epic-1-feature.md", + "jira_key": "T-100", + "title": "Feature", + "size": "M", + "content_hash": "abc", + "synced_status": "active", + "stories": [ + {"file": "story-01-api.md", "jira_key": "T-101", + "title": "[DEV] API", "content_hash": "d1", + "synced_status": "active"}, + {"file": "story-02-webhook.md", "jira_key": "T-102", + "title": "[DEV] Webhook", "content_hash": "d2", + "synced_status": "active"}, + {"file": "story-03-docs.md", "jira_key": "T-103", + "title": "[DOCS] API reference", "content_hash": content_hash, + "synced_status": "active"}, + ], + }]) + plan = sync_helper.detect_changes(str(self.artifacts_dir)) + docs_items = [i for i in plan["items"] + if i["file"].endswith("story-03-docs.md")] + self.assertEqual(len(docs_items), 1) + self.assertEqual(docs_items[0]["category"], "unchanged", + "Checkpointed DOCS with matching hash must be unchanged") + + # -- Instruction contract: sync.md guard text -- + + def test_sync_guard_covers_creation_and_update(self) -> None: + """sync.md guard applies to creation (6a), update (6b), and reopening.""" + sync_path = Path(__file__).resolve().parent.parent / "skills" / "sync.md" + sync_md = sync_path.read_text(encoding="utf-8") + guard = sync_md[sync_md.find("Reference-availability guard"): + sync_md.find("### Step 5:")] + self.assertIn("creation (Step 6a)", guard) + self.assertIn("update (Step 6b)", guard) + self.assertIn("reopening (Step 6b)", guard) + # Guard text wraps across lines; normalize whitespace for match + guard_flat = " ".join(guard.split()) + self.assertIn("do not record a manifest checkpoint", guard_flat, + "Guard must prohibit checkpoint for deferred DOCS") + + def test_docs_filename_identity_stable(self) -> None: + """DOCS story filename is immutable — not renumbered.""" + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" + self.assertTrue(docs_path.exists()) + self.assertEqual(docs_path.name, "story-03-docs.md") if __name__ == "__main__": From 3e6e4501821c49fa692529dd2623bf10678aa7e5 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 21:10:07 +0000 Subject: [PATCH 08/12] Address seven review findings: placement rubric, guard callsites, deferral policy, tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1: Add DOCS placement check to decomposition-review criterion 8 as a conditional LOW finding — reviewer flags observable ordering; decomposer verifies sync identity before acting. Preserve canonical criterion 7 and reviewer independence. 2: Align MEDIUM severity evidence boundary to "PRD and artifact context" matching the rubric text. 3: Assert .find marker existence and correct order before slicing sync.md guard/callsite text in tests. Missing markers now cause clear assertion failures, not wrong-block slicing. 4: Replace both tautological filename-identity tests with meaningful detect_changes/manifest-identity checks: synced DOCS with late contributor stays unchanged, changed DOCS classified correctly. Add type annotation to _manifest_with_keys. 5: Rename stale #### 4a/4b → 3a/3b under decompose Step 3. 6: Add explicit reference-availability guard calls at 6a (create) and 6b (update/reopen) BEFORE prepare/render/Jira-write/checkpoint. Add callsite contract tests verifying guard reference and defer language in both sections. 7: Define bounded deferral: no retry in current /sync run; continue per-epic passes; record deferred file/keys/reason; report in Step 8 including deferred count and no-complete-sync warning. Next /sync invocation picks them up as new/changed. Add deferral-policy and Step 8 report contract tests. Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 21 ++-- design/scripts/test_sync_helper.py | 192 +++++++++++++++++++++++++---- design/skills/decompose.md | 4 +- design/skills/sync.md | 43 +++++-- 4 files changed, 215 insertions(+), 45 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index 01e18402..731dda22 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -159,12 +159,17 @@ impact: items that appear speculative or unresolved? Does the Design Reference include a `Contributing stories` line consistent with the Documentation Inputs headers? Are the contributing story references - complete? Note: the reviewer cannot verify whether unmarked - Documentation Inputs items are truly derived from the design document - or PRD — that provenance check belongs to the decomposer (see - the provenance verification in `/decompose`). Flag items that - appear speculative or unresolved from the PRD and artifact context - alone. + complete? Is each `[DOCS]` story's file number higher than all + its contributing stories' file numbers within the same epic (per + criterion 7 placement)? If a `[DOCS]` story appears to precede a + contributor, flag it as a conditional LOW finding — the decomposer + will verify whether it is a manifest-tracked post-sync story with + an immutable filename before acting on the finding. Note: the + reviewer cannot verify whether unmarked Documentation Inputs items + are truly derived from the design document or PRD — that provenance + check belongs to the decomposer (see the provenance verification + in `/decompose`). Flag items that appear speculative or unresolved + from the PRD and artifact context alone. ## Finding Format @@ -193,8 +198,8 @@ Reference section. |----------|---------|--------| | CRITICAL | Would cause incorrect implementation — missing requirements, contradictory acceptance criteria, broken dependency chain, scope reduction, stories referencing non-existent ICs or TCs, ICs with no implementing story, prerequisite story without justification or dependency declaration, `[DOCS]` story missing contributing stories or with Documentation Inputs that omit user-facing artifacts from referenced implementation stories | Must fix before presenting | | HIGH | Would cause rework — story too large to review in a single PR, missing testing commitment, integration gap between stories, `[QE]` story duplicating `[DEV]` test scope, stories missing Interface Changes or Validated by references, prerequisite story missing preserved-behavior tests, `[DOCS]` story with generic title or unidentified reader/task | Should fix before presenting | -| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story with Documentation Inputs items that appear speculative or unresolved from PRD context but lack `[Assumption]` or `[Open: §9.N]` markers (items verifiable against the PRD need no marker; the decomposer verifies design-derived provenance separately in the provenance sub-step of Step 8) | Fix if it adds value | -| LOW | Structural improvements — naming consistency, ordering suggestions, documentation clarity | Fix only if clearly valuable | +| MEDIUM | Would reduce clarity — vague acceptance criteria, imprecise implementation guidance or documentation scope, sizing concerns, minor coverage matrix inconsistencies, minor IC or TC reference inconsistencies, `[DOCS]` story with Documentation Inputs items that appear speculative or unresolved from PRD and artifact context but lack `[Assumption]` or `[Open: §9.N]` markers (items verifiable against the PRD need no marker; the decomposer verifies design-derived provenance separately in the provenance sub-step of Step 8) | Fix if it adds value | +| LOW | Structural improvements — naming consistency, ordering suggestions, documentation clarity, `[DOCS]` story placement that appears to precede a contributor (conditional — decomposer verifies sync identity before acting) | Fix only if clearly valuable | ## Validation Rules diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index 5b9676d9..fa9fcb9a 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4164,12 +4164,37 @@ def test_cross_epic_contributor_resolves_after_creation(self) -> None: self.assertIn("EDM-201", desc) self.assertNotIn("[unresolved]", desc) - def test_docs_filename_identity_stable(self) -> None: - """DOCS story filename/number is immutable in the manifest.""" + def test_synced_docs_identity_stable_with_late_contributor(self) -> None: + """Synced DOCS story-03 retains its manifest identity when + story-04 is a later contributor — detect_changes classifies + it as unchanged, not new or removed.""" docs_path = self.stories_dir / "epic-1" / "story-03-rollback-docs.md" - self.assertTrue(docs_path.exists()) - # The file is story-03, not story-05 — it was not renumbered - self.assertEqual(docs_path.name, "story-03-rollback-docs.md") + content_hash = sync_helper.compute_file_hash(str(docs_path)) + artifacts_dir = Path(self.tmpdir) + _make_manifest(artifacts_dir, [{ + "file": "epic-1-rollback.md", + "jira_key": "EDM-100", + "title": "Rollback", "size": "M", + "content_hash": "x", "synced_status": "active", + "stories": [ + {"file": "story-01-rollback-cmd.md", "jira_key": "EDM-101", + "title": "[DEV] Rollback cmd", "content_hash": "y", + "synced_status": "active"}, + {"file": "story-03-rollback-docs.md", "jira_key": "EDM-103", + "title": "[DOCS] Rollback CLI reference", + "content_hash": content_hash, + "synced_status": "active"}, + {"file": "story-04-dry-run-flag.md", "jira_key": "EDM-104", + "title": "[DEV] Dry-run flag", "content_hash": "z", + "synced_status": "active"}, + ], + }]) + plan = sync_helper.detect_changes(str(artifacts_dir)) + docs_items = [i for i in plan["items"] + if i["file"].endswith("story-03-rollback-docs.md")] + self.assertEqual(len(docs_items), 1) + self.assertEqual(docs_items[0]["category"], "unchanged") + self.assertEqual(docs_items[0]["jira_key"], "EDM-103") # --------------------------------------------------------------------------- @@ -4323,7 +4348,7 @@ def setUp(self) -> None: doc_inputs="**Story 1.01:** endpoint.\n" "**Story 1.02:** webhook.") - def _manifest_with_keys(self, story_keys): + def _manifest_with_keys(self, story_keys: list[dict]) -> dict: """Build a manifest with the given story entries.""" return {"epics": [{ "file": "epic-1-feature.md", @@ -4451,27 +4476,146 @@ def test_successful_checkpoint_makes_docs_unchanged(self) -> None: self.assertEqual(docs_items[0]["category"], "unchanged", "Checkpointed DOCS with matching hash must be unchanged") - # -- Instruction contract: sync.md guard text -- + # -- Instruction contracts: sync.md guard and callsite text -- - def test_sync_guard_covers_creation_and_update(self) -> None: - """sync.md guard applies to creation (6a), update (6b), and reopening.""" + @classmethod + def _read_sync_md(cls) -> str: sync_path = Path(__file__).resolve().parent.parent / "skills" / "sync.md" - sync_md = sync_path.read_text(encoding="utf-8") - guard = sync_md[sync_md.find("Reference-availability guard"): - sync_md.find("### Step 5:")] - self.assertIn("creation (Step 6a)", guard) - self.assertIn("update (Step 6b)", guard) - self.assertIn("reopening (Step 6b)", guard) - # Guard text wraps across lines; normalize whitespace for match - guard_flat = " ".join(guard.split()) - self.assertIn("do not record a manifest checkpoint", guard_flat, - "Guard must prohibit checkpoint for deferred DOCS") - - def test_docs_filename_identity_stable(self) -> None: - """DOCS story filename is immutable — not renumbered.""" + return sync_path.read_text(encoding="utf-8") + + def test_sync_guard_covers_creation_and_update(self) -> None: + """Step 4 guard covers creation (6a), update (6b), and reopening.""" + sync_md = self._read_sync_md() + start = sync_md.find("Reference-availability guard") + end = sync_md.find("### Step 5:") + self.assertGreaterEqual(start, 0, "Guard section must exist") + self.assertGreater(end, start, "Step 5 must follow guard") + guard_flat = " ".join(sync_md[start:end].split()) + self.assertIn("creation (Step 6a)", guard_flat) + self.assertIn("update (Step 6b)", guard_flat) + self.assertIn("reopening (Step 6b)", guard_flat) + self.assertIn("do not record a manifest checkpoint", guard_flat) + + def test_6a_callsite_references_guard(self) -> None: + """Step 6a explicitly applies the reference-availability guard + before prepare for [DOCS] stories.""" + sync_md = self._read_sync_md() + start = sync_md.find("#### 6a:") + end = sync_md.find("#### 6b:") + self.assertGreaterEqual(start, 0, "Step 6a must exist") + self.assertGreater(end, start, "Step 6b must follow 6a") + section = " ".join(sync_md[start:end].split()) + self.assertIn("reference-availability guard", section, + "6a must reference the guard before prepare") + self.assertIn("defer", section, + "6a must defer DOCS with missing keys") + + def test_6b_callsite_references_guard(self) -> None: + """Step 6b explicitly applies the reference-availability guard + before prepare for changed/reopened [DOCS] stories.""" + sync_md = self._read_sync_md() + start = sync_md.find("#### 6b:") + end = sync_md.find("#### 6c:") + self.assertGreaterEqual(start, 0, "Step 6b must exist") + self.assertGreater(end, start, "Step 6c must follow 6b") + section = " ".join(sync_md[start:end].split()) + self.assertIn("reference-availability guard", section, + "6b must reference the guard before prepare") + self.assertIn("defer", section) + + def test_deferral_no_current_run_retry(self) -> None: + """Guard deferral policy: no retry in current /sync invocation.""" + sync_md = self._read_sync_md() + guard_flat = " ".join(sync_md.split()) + self.assertIn("Do not retry a deferred", guard_flat) + self.assertIn("current `/sync` invocation", guard_flat) + + def test_step8_reports_deferred(self) -> None: + """Step 8 report includes deferred DOCS stories.""" + sync_md = self._read_sync_md() + start = sync_md.find("### Step 8:") + self.assertGreaterEqual(start, 0, "Step 8 must exist") + section = sync_md[start:] + self.assertIn("deferred", section) + self.assertIn("contributor keys were missing", section) + self.assertIn("Do not claim complete synchronization", section) + + # -- Manifest identity: synced DOCS stable through detect_changes -- + + def test_synced_docs_with_late_contributor_stays_unchanged(self) -> None: + """A synced DOCS story retains its manifest identity when a + higher-numbered contributor is added. detect_changes does not + classify it as new, removed, or renamed.""" + # story-04 is a late contributor added after DOCS story-03 was synced + _make_dev_story(self.stories_dir, 1, 4, "metrics", title="Add metrics") docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" - self.assertTrue(docs_path.exists()) - self.assertEqual(docs_path.name, "story-03-docs.md") + content_hash = sync_helper.compute_file_hash(str(docs_path)) + _make_manifest(self.artifacts_dir, [{ + "file": "epic-1-feature.md", + "jira_key": "T-100", + "title": "Feature", "size": "M", + "content_hash": "abc", "synced_status": "active", + "stories": [ + {"file": "story-01-api.md", "jira_key": "T-101", + "title": "[DEV] API", "content_hash": "d1", + "synced_status": "active"}, + {"file": "story-02-webhook.md", "jira_key": "T-102", + "title": "[DEV] Webhook", "content_hash": "d2", + "synced_status": "active"}, + {"file": "story-03-docs.md", "jira_key": "T-103", + "title": "[DOCS] API reference", + "content_hash": content_hash, + "synced_status": "active"}, + # story-04 is new (not yet in manifest) + ], + }]) + plan = sync_helper.detect_changes(str(self.artifacts_dir)) + docs_items = [i for i in plan["items"] + if i["file"].endswith("story-03-docs.md")] + self.assertEqual(len(docs_items), 1) + self.assertEqual(docs_items[0]["category"], "unchanged", + "Synced DOCS must stay unchanged when late contributor added") + self.assertEqual(docs_items[0]["jira_key"], "T-103", + "Synced DOCS must retain its Jira key") + # story-04 should be new + new_items = [i for i in plan["items"] + if i["file"].endswith("story-04-metrics.md")] + self.assertEqual(len(new_items), 1) + self.assertEqual(new_items[0]["category"], "new") + + def test_synced_docs_changed_when_inputs_updated(self) -> None: + """A synced DOCS story whose content changed (e.g., updated + Documentation Inputs) is correctly classified as changed.""" + docs_path = self.stories_dir / "epic-1" / "story-03-docs.md" + old_hash = sync_helper.compute_file_hash(str(docs_path)) + _make_manifest(self.artifacts_dir, [{ + "file": "epic-1-feature.md", + "jira_key": "T-100", + "title": "Feature", "size": "M", + "content_hash": "abc", "synced_status": "active", + "stories": [ + {"file": "story-01-api.md", "jira_key": "T-101", + "title": "[DEV] API", "content_hash": "d1", + "synced_status": "active"}, + {"file": "story-02-webhook.md", "jira_key": "T-102", + "title": "[DEV] Webhook", "content_hash": "d2", + "synced_status": "active"}, + {"file": "story-03-docs.md", "jira_key": "T-103", + "title": "[DOCS] API reference", + "content_hash": old_hash, + "synced_status": "active"}, + ], + }]) + # Modify the DOCS story content + content = docs_path.read_text() + docs_path.write_text(content + "\nUpdated Documentation Inputs.\n") + plan = sync_helper.detect_changes(str(self.artifacts_dir)) + docs_items = [i for i in plan["items"] + if i["file"].endswith("story-03-docs.md")] + self.assertEqual(len(docs_items), 1) + self.assertEqual(docs_items[0]["category"], "changed", + "Modified DOCS must be classified as changed") + self.assertEqual(docs_items[0]["jira_key"], "T-103") if __name__ == "__main__": diff --git a/design/skills/decompose.md b/design/skills/decompose.md index ceeca3ee..4d317aff 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -90,7 +90,7 @@ This step produces two things: a metadata index (`05-epics.md`) and one file per epic. Each epic file has a 1:1 correspondence with a Jira Epic issue — its content is what `/sync` will push to Jira. -#### 4a: Write Epic Metadata +#### 3a: Write Epic Metadata Write `.artifacts/design/{issue-key}/05-epics.md`: @@ -127,7 +127,7 @@ Write `.artifacts/design/{issue-key}/05-epics.md`: that can proceed in parallel.} ``` -#### 4b: Write Epic Files +#### 3b: Write Epic Files For each epic, write `.artifacts/design/{issue-key}/06-stories/epic-{N}-{slug}.md`: diff --git a/design/skills/sync.md b/design/skills/sync.md index ba79c0c1..fdcefb5e 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -280,9 +280,13 @@ Dependencies has a Jira key in the manifest. If any required key is missing (contributor creation was skipped, failed, or is in a not-yet-processed epic), **defer** that `[DOCS]` write: do not render, create, or update the Jira issue, and do not record a -manifest checkpoint. The deferred `[DOCS]` story remains eligible -for retry after the missing contributor's epic completes or on a -subsequent `/sync` run. Report deferred stories to the user. +manifest checkpoint. Do not retry a deferred `[DOCS]` story later +in the current `/sync` invocation, even if a subsequent epic's +creation pass supplies the missing key. Continue processing +eligible items in the existing per-epic passes. Record each +deferred story's file, missing keys, and reason for the Step 8 +report. Because no checkpoint was saved, deferred stories remain +`new` or `changed` on the next explicit `/sync` invocation. If any `[DOCS]` story fails validation, present the findings and recommend `/revise` — sync must not modify story source files. Do @@ -506,10 +510,20 @@ story.** Instead, stop and present the match to the user with the same options as in Step 5a (skip and record, create anyway, or stop entirely). Wait for the user's choice before continuing. -For each new story, prepare the hash and description using the story -`prepare` recipe from the helper reference. `PREP` is JSON with -`content_hash`, `description`, `summary` (`[PREFIX] title` without -numbering), `story_type`, `dependencies` (resolved text), and `title`. +**`[DOCS]` reference-availability guard.** Before preparing a `[DOCS]` +story, apply the reference-availability guard from Step 4: verify that +every story in its Contributing stories and Dependencies has a Jira key +in the manifest. If any key is missing, **defer** this story — skip +`prepare`, Jira creation, and manifest checkpoint. Record the story +file, missing keys, and reason for the Step 8 deferral report. Continue +with the next story in this epic. The deferred story remains eligible +for the next `/sync` invocation; do not retry it in the current run. + +For each new story (non-deferred), prepare the hash and description +using the story `prepare` recipe from the helper reference. `PREP` is +JSON with `content_hash`, `description`, `summary` (`[PREFIX] title` +without numbering), `story_type`, `dependencies` (resolved text), and +`title`. Create the Jira issue using fields from `PREP`: @@ -587,9 +601,15 @@ so re-running `/sync` later will only attempt uncreated items. #### 6b: Update Changed Stories -For each story categorized as **Changed** or **Reopened**, prepare the -updated description and hash using the story `prepare` recipe from the -helper reference. +**`[DOCS]` reference-availability guard.** Before preparing a changed +or reopened `[DOCS]` story, apply the same reference-availability guard +as in Step 6a. If any required contributor key is missing, defer this +story and record it for the Step 8 deferral report. Do not retry it in +the current run. + +For each story categorized as **Changed** or **Reopened** (non-deferred), +prepare the updated description and hash using the story `prepare` +recipe from the helper reference. Update the Jira issue using fields from `PREP`: @@ -688,7 +708,8 @@ Fields: ### Step 8: Report to User Summarize: -- How many epics and stories were created, updated, closed, and unchanged +- How many epics and stories were created, updated, closed, unchanged, and **deferred** +- For each deferred `[DOCS]` story: the story file, which contributor keys were missing, and the reason. Note that these stories will appear as `new` or `changed` on the next `/sync` run because no checkpoint was recorded. Do not claim complete synchronization while deferred stories remain - Confirm the hierarchy was verified: every epic has parent = Feature (verified in Step 5), every story has parent = its epic (verified in Step 6) - Link to the Feature issue in Jira (which now shows the full hierarchy) From 9b30d6e7f7d3419688dc871c572869711f702500 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 21:18:53 +0000 Subject: [PATCH 09/12] Strengthen placement rubric, guard ordering proofs, deferral completeness - Review criterion 8: cover cross-cutting placement (last-epic) per canonical criterion 7; conditional LOW with manifest-tracked exception. - decompose Step 8: explicit decomposer-side sync-manifest identity check before acting on placement findings; request verification when identity context unavailable. - Sync Step 8 report: qualify hierarchy verification for issues actually created/updated; deferred stories have no Jira parent to verify. - Test 6a/6b guard: prove guard precedes actual prepare recipe instruction (position assertion), not just word presence. Verify skip-prepare semantics. - Add placement scenario/identity contract tests: cross-cutting rubric coverage, decomposer manifest-check instruction, review-rubric text. Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 13 +++--- design/scripts/test_sync_helper.py | 65 ++++++++++++++++++++++++------ design/skills/decompose.md | 10 +++++ design/skills/sync.md | 2 +- 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index 731dda22..61e3a5b8 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -159,12 +159,13 @@ impact: items that appear speculative or unresolved? Does the Design Reference include a `Contributing stories` line consistent with the Documentation Inputs headers? Are the contributing story references - complete? Is each `[DOCS]` story's file number higher than all - its contributing stories' file numbers within the same epic (per - criterion 7 placement)? If a `[DOCS]` story appears to precede a - contributor, flag it as a conditional LOW finding — the decomposer - will verify whether it is a manifest-tracked post-sync story with - an immutable filename before acting on the finding. Note: the + complete? Per criterion 7 placement: for per-epic `[DOCS]` stories, + is the file number higher than all its contributing stories within + the same epic? For cross-cutting `[DOCS]` stories, is it placed as + the last story in the last epic? If a `[DOCS]` story appears to + violate either rule, flag it as a conditional LOW finding — the + decomposer will verify whether it is a manifest-tracked post-sync + story with an immutable filename before acting. Note: the reviewer cannot verify whether unmarked Documentation Inputs items are truly derived from the design document or PRD — that provenance check belongs to the decomposer (see the provenance verification diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index fa9fcb9a..d311cc81 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4320,6 +4320,33 @@ def test_open_question_design_references_preserved(self) -> None: not decompose step numbers.""" self.assertIn("[Open: §9.N]", self._decompose_md) + def test_placement_identity_check_in_step8(self) -> None: + """Step 8 instructs decomposer to check sync-manifest identity + before acting on conditional placement findings.""" + step8 = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + manifest_idx = step8.find("sync-manifest.json") + self.assertGreaterEqual(manifest_idx, 0, + "Step 8 must reference sync-manifest.json") + self.assertIn("manifest-tracked", step8) + self.assertIn("request verification", step8, + "Must request verification when identity uncertain") + + # -- Placement scenarios (contract: review rubric text) -- + + def test_review_rubric_covers_cross_cutting_placement(self) -> None: + """Criterion 8 covers cross-cutting DOCS placement (last epic) + using canonical criterion 7 wording.""" + review_path = Path(__file__).resolve().parent.parent / "decomposition-review.md" + review_md = review_path.read_text(encoding="utf-8") + crit8_start = review_md.find("8. **Documentation**") + self.assertGreaterEqual(crit8_start, 0) + crit8 = review_md[crit8_start:review_md.find("## Finding Format")] + self.assertIn("last story in the last epic", crit8, + "Must cover cross-cutting placement per criterion 7") + self.assertIn("conditional LOW", crit8) + self.assertIn("manifest-tracked", crit8, + "Must note post-sync exception") + # --------------------------------------------------------------------------- # Sync: reference-availability guard for DOCS creation and update @@ -4496,31 +4523,45 @@ def test_sync_guard_covers_creation_and_update(self) -> None: self.assertIn("reopening (Step 6b)", guard_flat) self.assertIn("do not record a manifest checkpoint", guard_flat) - def test_6a_callsite_references_guard(self) -> None: - """Step 6a explicitly applies the reference-availability guard - before prepare for [DOCS] stories.""" + def test_6a_guard_precedes_prepare(self) -> None: + """Step 6a: reference-availability guard appears before the + story prepare recipe instruction.""" sync_md = self._read_sync_md() start = sync_md.find("#### 6a:") end = sync_md.find("#### 6b:") self.assertGreaterEqual(start, 0, "Step 6a must exist") self.assertGreater(end, start, "Step 6b must follow 6a") - section = " ".join(sync_md[start:end].split()) - self.assertIn("reference-availability guard", section, - "6a must reference the guard before prepare") + section = sync_md[start:end] + guard_pos = section.find("reference-availability guard") + prepare_pos = section.find("`prepare` recipe") + self.assertGreaterEqual(guard_pos, 0, + "6a must contain reference-availability guard") + self.assertGreaterEqual(prepare_pos, 0, + "6a must contain prepare recipe instruction") + self.assertLess(guard_pos, prepare_pos, + "Guard must precede prepare recipe in 6a") self.assertIn("defer", section, "6a must defer DOCS with missing keys") + self.assertIn("skip `prepare`", " ".join(section.split()), + "6a deferral must skip prepare") - def test_6b_callsite_references_guard(self) -> None: - """Step 6b explicitly applies the reference-availability guard - before prepare for changed/reopened [DOCS] stories.""" + def test_6b_guard_precedes_prepare(self) -> None: + """Step 6b: reference-availability guard appears before the + story prepare recipe instruction.""" sync_md = self._read_sync_md() start = sync_md.find("#### 6b:") end = sync_md.find("#### 6c:") self.assertGreaterEqual(start, 0, "Step 6b must exist") self.assertGreater(end, start, "Step 6c must follow 6b") - section = " ".join(sync_md[start:end].split()) - self.assertIn("reference-availability guard", section, - "6b must reference the guard before prepare") + section = sync_md[start:end] + guard_pos = section.find("reference-availability guard") + prepare_pos = section.find("`prepare`") + self.assertGreaterEqual(guard_pos, 0, + "6b must contain reference-availability guard") + self.assertGreaterEqual(prepare_pos, 0, + "6b must contain prepare instruction") + self.assertLess(guard_pos, prepare_pos, + "Guard must precede prepare in 6b") self.assertIn("defer", section) def test_deferral_no_current_run_retry(self) -> None: diff --git a/design/skills/decompose.md b/design/skills/decompose.md index 4d317aff..ac20a0ea 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -589,6 +589,16 @@ For each finding from Step 7: Only fix findings that add real value. Do not make changes for structural preferences not grounded in the evaluation criteria. +**Placement findings (conditional LOW).** If the review flagged a +`[DOCS]` story placement issue, check whether +`.artifacts/design/{issue-key}/sync-manifest.json` exists and contains +the story's filename. If the story is manifest-tracked (post-sync), +it retains its immutable filename and number — dismiss the placement +finding. If no manifest exists or the story is not tracked, apply the +placement fix. If the manifest's existence is uncertain (e.g., the +file was not loaded in Step 1), request verification from the user +rather than assuming the story is unsynced. + #### Re-review (if fixes were made) If the validation above produced changes to the decomposition artifacts: diff --git a/design/skills/sync.md b/design/skills/sync.md index fdcefb5e..ec4bf234 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -710,7 +710,7 @@ Fields: Summarize: - How many epics and stories were created, updated, closed, unchanged, and **deferred** - For each deferred `[DOCS]` story: the story file, which contributor keys were missing, and the reason. Note that these stories will appear as `new` or `changed` on the next `/sync` run because no checkpoint was recorded. Do not claim complete synchronization while deferred stories remain -- Confirm the hierarchy was verified: every epic has parent = Feature (verified in Step 5), every story has parent = its epic (verified in Step 6) +- Confirm the hierarchy was verified for every issue actually created or updated: every epic has parent = Feature (verified in Step 5), every story has parent = its epic (verified in Step 6). Deferred stories were not created and have no Jira parent to verify - Link to the Feature issue in Jira (which now shows the full hierarchy) **Do not suggest manual parent linking as a next step.** If any parent From 35a86cd76234caaebb22e38190c4956d37b5dafb Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 21:26:06 +0000 Subject: [PATCH 10/12] Fix deferral reporting, reopened state, and placement identity precision - Step 8: distinguish deferred new (not created in Jira) from deferred changed/reopened (exist but not updated). Do not assert deferred existing issues lack parents. - Deferral next-run: include `reopened` alongside `new`/`changed` since no checkpoint clears the reopened state. - Placement identity: match story filename within its parent epic's story list (epic-scoped, not basename search). Distinguish confirmed manifest absence (apply initial placement) from unreadable/ambiguous manifest (request verification). - Add contract tests for reopened deferral wording and Step 8 new/changed/reopened distinction. Co-Authored-By: Claude Opus 4.6 --- design/scripts/test_sync_helper.py | 11 ++++++++++- design/skills/decompose.md | 18 +++++++++++------- design/skills/sync.md | 7 ++++--- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index d311cc81..b794f746 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4572,7 +4572,8 @@ def test_deferral_no_current_run_retry(self) -> None: self.assertIn("current `/sync` invocation", guard_flat) def test_step8_reports_deferred(self) -> None: - """Step 8 report includes deferred DOCS stories.""" + """Step 8 report: deferred stories, missing keys, no-complete-sync, + and distinguishes deferred new from deferred changed/reopened.""" sync_md = self._read_sync_md() start = sync_md.find("### Step 8:") self.assertGreaterEqual(start, 0, "Step 8 must exist") @@ -4580,6 +4581,14 @@ def test_step8_reports_deferred(self) -> None: self.assertIn("deferred", section) self.assertIn("contributor keys were missing", section) self.assertIn("Do not claim complete synchronization", section) + self.assertIn("Deferred new stories were not created", section) + self.assertIn("deferred changed or reopened", section) + + def test_deferral_next_run_includes_reopened(self) -> None: + """Deferral wording includes reopened alongside new/changed.""" + sync_md = self._read_sync_md() + flat = " ".join(sync_md.split()) + self.assertIn("`new`, `changed`, or `reopened`", flat) # -- Manifest identity: synced DOCS stable through detect_changes -- diff --git a/design/skills/decompose.md b/design/skills/decompose.md index ac20a0ea..c44940b4 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -591,13 +591,17 @@ structural preferences not grounded in the evaluation criteria. **Placement findings (conditional LOW).** If the review flagged a `[DOCS]` story placement issue, check whether -`.artifacts/design/{issue-key}/sync-manifest.json` exists and contains -the story's filename. If the story is manifest-tracked (post-sync), -it retains its immutable filename and number — dismiss the placement -finding. If no manifest exists or the story is not tracked, apply the -placement fix. If the manifest's existence is uncertain (e.g., the -file was not loaded in Step 1), request verification from the user -rather than assuming the story is unsynced. +`.artifacts/design/{issue-key}/sync-manifest.json` exists. If it +does, look up the story by matching its filename within its parent +epic's story list in the manifest (the manifest nests stories under +their epic entry, so the same filename in different epics is +unambiguous). If the story is manifest-tracked (post-sync), it +retains its immutable filename and number — dismiss the placement +finding. If no manifest exists or the story is not in the manifest, +apply the placement fix (initial placement rules apply). If the +manifest could not be read (e.g., file not loaded in Step 1 or +I/O error), request verification from the user rather than assuming +the story is unsynced. #### Re-review (if fixes were made) diff --git a/design/skills/sync.md b/design/skills/sync.md index ec4bf234..f28d9e5b 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -286,7 +286,8 @@ creation pass supplies the missing key. Continue processing eligible items in the existing per-epic passes. Record each deferred story's file, missing keys, and reason for the Step 8 report. Because no checkpoint was saved, deferred stories remain -`new` or `changed` on the next explicit `/sync` invocation. +`new`, `changed`, or `reopened` (as appropriate) on the next +explicit `/sync` invocation. If any `[DOCS]` story fails validation, present the findings and recommend `/revise` — sync must not modify story source files. Do @@ -709,8 +710,8 @@ Fields: Summarize: - How many epics and stories were created, updated, closed, unchanged, and **deferred** -- For each deferred `[DOCS]` story: the story file, which contributor keys were missing, and the reason. Note that these stories will appear as `new` or `changed` on the next `/sync` run because no checkpoint was recorded. Do not claim complete synchronization while deferred stories remain -- Confirm the hierarchy was verified for every issue actually created or updated: every epic has parent = Feature (verified in Step 5), every story has parent = its epic (verified in Step 6). Deferred stories were not created and have no Jira parent to verify +- For each deferred `[DOCS]` story: the story file, which contributor keys were missing, and the reason. Note that these stories will appear as `new`, `changed`, or `reopened` (as appropriate) on the next `/sync` run because no checkpoint was recorded. Do not claim complete synchronization while deferred stories remain +- Confirm the hierarchy was verified for every issue actually created or updated: every epic has parent = Feature (verified in Step 5), every story has parent = its epic (verified in Step 6). Deferred new stories were not created in Jira; deferred changed or reopened stories exist in Jira but were not updated in this run. List deferred stories separately as work not applied - Link to the Feature issue in Jira (which now shows the full hierarchy) **Do not suggest manual parent linking as a next step.** If any parent From 35b20b74eb5bb13983d2c6ab972c5530117a861c Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 21:50:00 +0000 Subject: [PATCH 11/12] Align placement to full last-story rule, integrate identity gate, extend exception to dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A: Review criterion 8 placement uses full last-story-in-epic rule including non-contributing QE/CI stories, matching canonical criterion 7. Cross-cutting last-epic rule preserved. B: Move manifest identity check inside Step 8 per-finding value assessment — placement findings check sync identity BEFORE fix, not after all findings. Generic non-placement fixes proceed immediately. Manifest-tracked→dismiss, absent→initial placement, unreadable→request verification. C: Post-sync exemption and sync gate cover later-numbered prerequisite dependencies alongside contributors. Key check verifies Contributing stories AND Dependencies. 6b guard wording aligned ("required key" not "contributor key"). Add 6 contract tests: full last-story rule with QE/CI, identity gate before generic fix ordering, criterion 7 dependency coverage, non-contributing dependency resolution and deferral, sync exception consistency. Co-Authored-By: Claude Opus 4.6 --- design/decomposition-review.md | 15 +++-- design/scripts/test_sync_helper.py | 103 +++++++++++++++++++++++++++++ design/skills/decompose.md | 40 ++++++----- design/skills/sync.md | 15 +++-- 4 files changed, 138 insertions(+), 35 deletions(-) diff --git a/design/decomposition-review.md b/design/decomposition-review.md index 61e3a5b8..bb606521 100644 --- a/design/decomposition-review.md +++ b/design/decomposition-review.md @@ -159,13 +159,14 @@ impact: items that appear speculative or unresolved? Does the Design Reference include a `Contributing stories` line consistent with the Documentation Inputs headers? Are the contributing story references - complete? Per criterion 7 placement: for per-epic `[DOCS]` stories, - is the file number higher than all its contributing stories within - the same epic? For cross-cutting `[DOCS]` stories, is it placed as - the last story in the last epic? If a `[DOCS]` story appears to - violate either rule, flag it as a conditional LOW finding — the - decomposer will verify whether it is a manifest-tracked post-sync - story with an immutable filename before acting. Note: the + complete? Per criterion 7 placement: is each per-epic `[DOCS]` + story the last story in its epic (after all other stories, + including non-contributing `[QE]`/`[CI]` stories)? Is each + cross-cutting `[DOCS]` story the last story in the last epic? If + a `[DOCS]` story appears to violate either rule, flag it as a + conditional LOW finding — the decomposer will verify whether it + is a manifest-tracked post-sync story with an immutable filename + before acting. Note: the reviewer cannot verify whether unmarked Documentation Inputs items are truly derived from the design document or PRD — that provenance check belongs to the decomposer (see the provenance verification diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index b794f746..64e1ddd8 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4347,6 +4347,43 @@ def test_review_rubric_covers_cross_cutting_placement(self) -> None: self.assertIn("manifest-tracked", crit8, "Must note post-sync exception") + def test_review_rubric_full_last_story_rule(self) -> None: + """Criterion 8 uses the full last-story rule including non- + contributing QE/CI stories, not just higher-than-contributors.""" + review_path = Path(__file__).resolve().parent.parent / "decomposition-review.md" + review_md = review_path.read_text(encoding="utf-8") + crit8_start = review_md.find("8. **Documentation**") + crit8 = review_md[crit8_start:review_md.find("## Finding Format")] + self.assertGreaterEqual(crit8_start, 0) + self.assertIn("last story in its epic", crit8, + "Must use full last-story-in-epic rule") + self.assertIn("[QE]", crit8, + "Must mention non-contributing QE stories") + + def test_identity_gate_inside_per_finding_before_fix(self) -> None: + """Step 8 placement identity check occurs inside per-finding + value assessment, BEFORE the generic fix instruction.""" + step8 = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + identity_pos = step8.find("manifest-tracked") + fix_pos = step8.find("For all other findings:") + self.assertGreaterEqual(identity_pos, 0, + "Identity gate must exist in Step 8") + self.assertGreaterEqual(fix_pos, 0, + "Generic fix instruction must exist") + self.assertLess(identity_pos, fix_pos, + "Identity gate must precede generic fix") + + def test_criterion7_exception_covers_dependencies(self) -> None: + """Post-sync exemption in criterion 7 covers later-numbered + prerequisite dependencies, not only contributors.""" + flat = " ".join(self._decompose_md.split()) + self.assertIn( + "whether a contributor or a legitimate prerequisite dependency", + flat, "Exception must cover dependencies") + self.assertIn( + "Contributing stories AND Dependencies", + flat, "Key check must cover both Contributing stories and Dependencies") + # --------------------------------------------------------------------------- # Sync: reference-availability guard for DOCS creation and update @@ -4667,6 +4704,72 @@ def test_synced_docs_changed_when_inputs_updated(self) -> None: "Modified DOCS must be classified as changed") self.assertEqual(docs_items[0]["jira_key"], "T-103") + def test_noncontributing_dependency_resolves_when_key_available(self) -> None: + """A later-numbered prerequisite dependency (not in Contributing + stories but in Dependencies) resolves when its key exists.""" + # story-03 DOCS depends on story-04 (prereq, not a contributor) + stories_dir = Path(self.tmpdir) / "06-stories" + _make_dev_story(stories_dir, 1, 4, "infra", title="Infra prereq") + # Rewrite DOCS to add story-04 as dependency only (not contributor) + docs_path = stories_dir / "epic-1" / "story-03-docs.md" + content = docs_path.read_text() + content = content.replace( + "Story 1.01, Story 1.02", # Dependencies line + "Story 1.01, Story 1.02, Story 1.04") + docs_path.write_text(content) + + manifest = self._manifest_with_keys([ + {"file": "story-01-api.md", "jira_key": "T-101"}, + {"file": "story-02-webhook.md", "jira_key": "T-102"}, + {"file": "story-04-infra.md", "jira_key": "T-104"}, + ]) + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + self.assertNotIn("[unresolved]", desc, + "Non-contributing dependency with key must resolve") + self.assertIn("T-104", desc) + + def test_noncontributing_dependency_missing_key_defers(self) -> None: + """A later-numbered prerequisite dependency without a key + produces [unresolved], triggering the deferral guard.""" + stories_dir = Path(self.tmpdir) / "06-stories" + _make_dev_story(stories_dir, 1, 4, "infra", title="Infra prereq") + docs_path = stories_dir / "epic-1" / "story-03-docs.md" + content = docs_path.read_text() + content = content.replace( + "Story 1.01, Story 1.02", + "Story 1.01, Story 1.02, Story 1.04") + docs_path.write_text(content) + + manifest = self._manifest_with_keys([ + {"file": "story-01-api.md", "jira_key": "T-101"}, + {"file": "story-02-webhook.md", "jira_key": "T-102"}, + # story-04 NOT in manifest — creation pending + ]) + desc = sync_helper.render_story_description( + docs_path, manifest, story_type="DOCS") + self.assertIn("[unresolved]", desc, + "Missing dependency key must produce [unresolved]") + + # -- Instruction contract: sync exception covers dependencies -- + + def test_sync_exception_covers_dependencies(self) -> None: + """Sync placement exception and guard name both contributors + and prerequisite dependencies.""" + sync_md = self._read_sync_md() + start = sync_md.find("criterion 7 (placement)") + end = sync_md.find("**Reference-availability guard") + self.assertGreaterEqual(start, 0, "Placement exception must exist") + self.assertGreater(end, start, "Guard must follow exception") + section = " ".join(sync_md[start:end].split()) + self.assertIn("prerequisite dependency", section) + # Guard covers both Contributing stories and Dependencies + guard_start = sync_md.find("**Reference-availability guard") + guard_end = sync_md.find("### Step 5:") + self.assertGreater(guard_end, guard_start) + guard = " ".join(sync_md[guard_start:guard_end].split()) + self.assertIn("Contributing stories and Dependencies", guard) + if __name__ == "__main__": unittest.main() diff --git a/design/skills/decompose.md b/design/skills/decompose.md index c44940b4..239ddddd 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -362,11 +362,11 @@ validation gate. epics, place it as the last story in the last epic. **Post-sync exemption:** once a `[DOCS]` story has been synced (its filename is in the manifest), it retains its immutable filename and number even - when a later contributor is appended with a higher story number. - `/sync` verifies that every referenced contributor's Jira key is - available in the manifest before rendering the `[DOCS]` story's - description; if a key is missing, the update is deferred until the - contributor is created. + when a later-numbered story — whether a contributor or a legitimate + prerequisite dependency — is appended after it. `/sync` verifies + that every story referenced in Contributing stories AND Dependencies + has a Jira key in the manifest before rendering the `[DOCS]` story's + description; if any key is missing, the update is deferred. **Story quality checks:** - For `[DEV]` stories: does this story add a capability AND testing for that capability? For a justified behavior-preserving prerequisite, does it restructure existing code AND include tests demonstrating preserved behavior? @@ -581,28 +581,26 @@ For each finding from Step 7: acceptance criteria, more complete coverage, better dependency ordering, more appropriate sizing) if the change were made? - - If yes: fix the issue immediately. Re-read the affected artifact - file before modifying it. Verify the fix after applying it. + - If yes — **for placement findings (conditional LOW):** before + correcting, check whether the story is manifest-tracked by + looking up its filename within its parent epic's story list in + `.artifacts/design/{issue-key}/sync-manifest.json`. If manifest- + tracked (post-sync), dismiss the finding — the story retains its + immutable filename and number. If no manifest exists or the story + is not tracked, the initial placement rules apply and the fix is + valid. If the manifest could not be read (e.g., not loaded in + Step 1 or I/O error), request verification from the user rather + than assuming unsynced. + + **For all other findings:** fix the issue immediately. Re-read + the affected artifact file before modifying it. Verify the fix + after applying it. - If no: note why the finding was dismissed (structural preference, no real improvement, would add churn without value). Only fix findings that add real value. Do not make changes for structural preferences not grounded in the evaluation criteria. -**Placement findings (conditional LOW).** If the review flagged a -`[DOCS]` story placement issue, check whether -`.artifacts/design/{issue-key}/sync-manifest.json` exists. If it -does, look up the story by matching its filename within its parent -epic's story list in the manifest (the manifest nests stories under -their epic entry, so the same filename in different epics is -unambiguous). If the story is manifest-tracked (post-sync), it -retains its immutable filename and number — dismiss the placement -finding. If no manifest exists or the story is not in the manifest, -apply the placement fix (initial placement rules apply). If the -manifest could not be read (e.g., file not loaded in Step 1 or -I/O error), request verification from the user rather than assuming -the story is unsynced. - #### Re-review (if fixes were made) If the validation above produced changes to the decomposition artifacts: diff --git a/design/skills/sync.md b/design/skills/sync.md index f28d9e5b..cbf9bbad 100644 --- a/design/skills/sync.md +++ b/design/skills/sync.md @@ -260,10 +260,11 @@ this from the change plan's full item list (new, changed, removed, reopened), not only from existing references. When evaluating criterion 7 (placement), synced `[DOCS]` stories -retain their immutable filename and number. If a contributor is -appended with a higher story number after the `[DOCS]` story was -synced, the `[DOCS]` story may numerically precede it — do not fail -this gate solely for that ordering. +retain their immutable filename and number. If a later-numbered story +— whether a contributor or a legitimate prerequisite dependency — is +appended after the `[DOCS]` story was synced, the `[DOCS]` story may +numerically precede it — do not fail this gate solely for that +ordering. **Reference-availability guard for `[DOCS]` stories.** Step 6 processes stories per epic in three passes (create, update, close). @@ -604,9 +605,9 @@ so re-running `/sync` later will only attempt uncreated items. **`[DOCS]` reference-availability guard.** Before preparing a changed or reopened `[DOCS]` story, apply the same reference-availability guard -as in Step 6a. If any required contributor key is missing, defer this -story and record it for the Step 8 deferral report. Do not retry it in -the current run. +as in Step 6a. If any required key is missing, defer this story and +record it for the Step 8 deferral report. Do not retry it in the +current run. For each story categorized as **Changed** or **Reopened** (non-deferred), prepare the updated description and hash using the story `prepare` From 92afc0a484d2f3ffea8b9392f2d7f0b75b4aa20e Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 30 Oct 2026 21:54:49 +0000 Subject: [PATCH 12/12] Fix placement control flow and test precision - Step 8 placement branch now explicitly applies the fix for unsynced stories (re-read/correct/verify) after the identity gate dismisses synced stories. Generic fix branch handles all non-placement findings. Both branches share the re-read/verify pattern. - Tests normalize whitespace before position assertions to handle markdown line wrapping. - Non-contributing dependency fixtures modify only Dependencies section (not Contributing stories or Documentation Inputs) with observable assertions proving isolation. Docstrings clarify renderer-only scope. - Review rubric test asserts both slice endpoints exist before indexing. Co-Authored-By: Claude Opus 4.6 --- design/scripts/test_sync_helper.py | 82 ++++++++++++++++++++---------- design/skills/decompose.md | 28 +++++----- 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/design/scripts/test_sync_helper.py b/design/scripts/test_sync_helper.py index 64e1ddd8..5a91241c 100644 --- a/design/scripts/test_sync_helper.py +++ b/design/scripts/test_sync_helper.py @@ -4323,10 +4323,10 @@ def test_open_question_design_references_preserved(self) -> None: def test_placement_identity_check_in_step8(self) -> None: """Step 8 instructs decomposer to check sync-manifest identity before acting on conditional placement findings.""" - step8 = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] - manifest_idx = step8.find("sync-manifest.json") - self.assertGreaterEqual(manifest_idx, 0, - "Step 8 must reference sync-manifest.json") + step8_raw = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + step8 = " ".join(step8_raw.split()) + self.assertIn("sync-manifest.json", step8, + "Step 8 must reference sync-manifest.json") self.assertIn("manifest-tracked", step8) self.assertIn("request verification", step8, "Must request verification when identity uncertain") @@ -4353,24 +4353,33 @@ def test_review_rubric_full_last_story_rule(self) -> None: review_path = Path(__file__).resolve().parent.parent / "decomposition-review.md" review_md = review_path.read_text(encoding="utf-8") crit8_start = review_md.find("8. **Documentation**") - crit8 = review_md[crit8_start:review_md.find("## Finding Format")] - self.assertGreaterEqual(crit8_start, 0) + crit8_end = review_md.find("## Finding Format") + self.assertGreaterEqual(crit8_start, 0, "Criterion 8 must exist") + self.assertGreater(crit8_end, crit8_start, + "Finding Format must follow criterion 8") + crit8 = review_md[crit8_start:crit8_end] self.assertIn("last story in its epic", crit8, "Must use full last-story-in-epic rule") self.assertIn("[QE]", crit8, "Must mention non-contributing QE stories") - def test_identity_gate_inside_per_finding_before_fix(self) -> None: - """Step 8 placement identity check occurs inside per-finding - value assessment, BEFORE the generic fix instruction.""" - step8 = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + def test_identity_gate_before_any_placement_fix(self) -> None: + """Step 8: placement identity check precedes both the unsynced + placement fix and the generic fix instruction.""" + step8_raw = self._decompose_md.split("### Step 8:")[1].split("### Step 9:")[0] + step8 = " ".join(step8_raw.split()) # normalize whitespace identity_pos = step8.find("manifest-tracked") - fix_pos = step8.find("For all other findings:") + placement_fix = step8.find("apply the initial placement fix") + generic_fix = step8.find("For all other findings:") self.assertGreaterEqual(identity_pos, 0, - "Identity gate must exist in Step 8") - self.assertGreaterEqual(fix_pos, 0, + "Identity gate must exist") + self.assertGreaterEqual(placement_fix, 0, + "Unsynced placement fix instruction must exist") + self.assertGreaterEqual(generic_fix, 0, "Generic fix instruction must exist") - self.assertLess(identity_pos, fix_pos, + self.assertLess(identity_pos, placement_fix, + "Identity gate must precede placement fix") + self.assertLess(identity_pos, generic_fix, "Identity gate must precede generic fix") def test_criterion7_exception_covers_dependencies(self) -> None: @@ -4705,18 +4714,31 @@ def test_synced_docs_changed_when_inputs_updated(self) -> None: self.assertEqual(docs_items[0]["jira_key"], "T-103") def test_noncontributing_dependency_resolves_when_key_available(self) -> None: - """A later-numbered prerequisite dependency (not in Contributing - stories but in Dependencies) resolves when its key exists.""" - # story-03 DOCS depends on story-04 (prereq, not a contributor) + """A later-numbered prerequisite dependency (in Dependencies but + NOT in Contributing stories/Documentation Inputs) resolves when + its manifest key exists. Renderer test only — does not test + live deferral/checkpoint behavior.""" stories_dir = Path(self.tmpdir) / "06-stories" _make_dev_story(stories_dir, 1, 4, "infra", title="Infra prereq") - # Rewrite DOCS to add story-04 as dependency only (not contributor) + # Add story-04 to Dependencies ONLY, not Contributing stories docs_path = stories_dir / "epic-1" / "story-03-docs.md" content = docs_path.read_text() - content = content.replace( - "Story 1.01, Story 1.02", # Dependencies line - "Story 1.01, Story 1.02, Story 1.04") + # Replace only in the Dependencies section (after "## Dependencies") + dep_marker = "## Dependencies\n\n" + dep_start = content.find(dep_marker) + self.assertGreater(dep_start, 0, "Dependencies section must exist") + dep_val_start = dep_start + len(dep_marker) + dep_line_end = content.find("\n", dep_val_start) + old_deps = content[dep_val_start:dep_line_end] + content = (content[:dep_val_start] + + old_deps + ", Story 1.04" + + content[dep_line_end:]) docs_path.write_text(content) + # Verify Contributing stories was NOT changed + self.assertNotIn("Story 1.04", + content.split("Contributing stories:")[1].split("\n")[0] + if "Contributing stories:" in content else "", + "Story 1.04 must not be in Contributing stories") manifest = self._manifest_with_keys([ {"file": "story-01-api.md", "jira_key": "T-101"}, @@ -4730,15 +4752,23 @@ def test_noncontributing_dependency_resolves_when_key_available(self) -> None: self.assertIn("T-104", desc) def test_noncontributing_dependency_missing_key_defers(self) -> None: - """A later-numbered prerequisite dependency without a key - produces [unresolved], triggering the deferral guard.""" + """A later-numbered prerequisite dependency without a manifest + key produces [unresolved] in the rendered description. The sync + agent uses this signal to defer the DOCS write (no Jira call or + manifest checkpoint). Renderer-only test — deferral/checkpoint + behavior is tested by state/contract tests.""" stories_dir = Path(self.tmpdir) / "06-stories" _make_dev_story(stories_dir, 1, 4, "infra", title="Infra prereq") docs_path = stories_dir / "epic-1" / "story-03-docs.md" content = docs_path.read_text() - content = content.replace( - "Story 1.01, Story 1.02", - "Story 1.01, Story 1.02, Story 1.04") + dep_marker = "## Dependencies\n\n" + dep_start = content.find(dep_marker) + dep_val_start = dep_start + len(dep_marker) + dep_line_end = content.find("\n", dep_val_start) + old_deps = content[dep_val_start:dep_line_end] + content = (content[:dep_val_start] + + old_deps + ", Story 1.04" + + content[dep_line_end:]) docs_path.write_text(content) manifest = self._manifest_with_keys([ diff --git a/design/skills/decompose.md b/design/skills/decompose.md index 239ddddd..71655f94 100644 --- a/design/skills/decompose.md +++ b/design/skills/decompose.md @@ -581,20 +581,20 @@ For each finding from Step 7: acceptance criteria, more complete coverage, better dependency ordering, more appropriate sizing) if the change were made? - - If yes — **for placement findings (conditional LOW):** before - correcting, check whether the story is manifest-tracked by - looking up its filename within its parent epic's story list in - `.artifacts/design/{issue-key}/sync-manifest.json`. If manifest- - tracked (post-sync), dismiss the finding — the story retains its - immutable filename and number. If no manifest exists or the story - is not tracked, the initial placement rules apply and the fix is - valid. If the manifest could not be read (e.g., not loaded in - Step 1 or I/O error), request verification from the user rather - than assuming unsynced. - - **For all other findings:** fix the issue immediately. Re-read - the affected artifact file before modifying it. Verify the fix - after applying it. + - If yes — **for `[DOCS]` placement findings (conditional LOW):** + before correcting, check whether the story is manifest-tracked + by looking up its filename within its parent epic's story list + in `.artifacts/design/{issue-key}/sync-manifest.json`. If + manifest-tracked (post-sync), dismiss the finding — the story + retains its immutable filename and number. If the manifest could + not be read (e.g., not loaded in Step 1 or I/O error), request + verification from the user rather than assuming unsynced. If no + manifest exists or the story is not tracked, apply the initial + placement fix: re-read the affected artifact file, correct the + placement, and verify the fix. + + **For all other findings:** re-read the affected artifact file, + fix the issue, and verify the fix. - If no: note why the finding was dismissed (structural preference, no real improvement, would add churn without value).