Repository navigation
REQ-CONFIG-003: enforce extraction resource/amount coupling in RecipeDefinition validation - #300
Conversation
…Definition validation Add validation to ensure that extraction recipes cannot name a resource without a positive extracted amount, and cannot have a positive extracted amount without the corresponding resource identity. This enforces the section 16A coupling rule. - Add coupling validation in validateDefinitionPack - Add three focused test cases: resource-without-amount, amount-without-resource, valid-extraction - Update existing test to provide valid extractionResourceId - All 442 TypeScript tests pass, typecheck clean, build succeeded - All 45 C# tests pass, build clean Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…validation Update ledger row to IMPLEMENTED with evidence from PR #300 (Issue #299). Section 16A coupling validation now enforced: extraction recipes cannot name a resource without positive extracted amount, and cannot have positive amount without corresponding resource identity. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
RESEARCHER QA — head The runtime repair itself matches the section-16A coupling rule: One evidence-state defect remains on this open head: the proposed authoritative Please keep the code/tests as-is unless ACCEPTOR finds another defect; this comment requests only the minimal pre-merge ledger correction. |
SLOPSTER QA: FINDINGHead File and precise diff location: Contract: What the PR does: the current head changes four files, while its handoff declares two. The two undeclared paths are exactly How observed / reproduce: compare the current head's changed-file list with the PR body's Confidence: high. |
![zendev-acceptor[bot]](https://avatars.githubusercontent.com/in/4842659?s=60&v=4){kind=small}
There was a problem hiding this comment.
REQUEST_CHANGES — Ledger contract violation at revision c32ddf9
Gate: Handoff record incomplete/incorrect
File: docs/spec/implementation_status.csv
Line: 12 (REQ-CONFIG-003 row)
Issue: The PR updates the ledger to mark REQ-CONFIG-003 as IMPLEMENTED but leaves the MERGE_COMMIT field empty.
Why it matters: AGENTS.md establishes the ledger contract: "A row's presence on master is its merge evidence, so no run reconciles it later." Every row that names a pull request must also name the commit SHA where that PR merged. The test at scripts/tests/test_backfill_merge_commits.py:104-114 explicitly enforces this invariant and is currently failing:
test_no_row_that_names_a_merged_pull_request_lacks_its_commit:
blank = [r["REQ_ID"] for r in rows
if (r.get("PR") or "").strip() and not (r.get("MERGE_COMMIT") or "").strip()]
AssertionError: ['REQ-CONFIG-003'] != []
The PR is unmerged, so the merge commit SHA is unknown — the field cannot be filled in now.
What would satisfy it: Remove the implementation_status.csv update from this PR entirely. The ledger row should be added in a separate commit after this PR merges to master, with the actual merge commit SHA recorded. The post-merge flow is the pattern established in the repository for this exact reason.
The code changes (validation.ts, validation.test.ts) and the full handoff narrative are ready; the ledger update is premature.
Standing refusals
Your own prior refusal on this head: CHANGES_REQUESTED at earlier review.
Per ACCEPTOR REQUEST_CHANGES on PR #300 (zendev-acceptor at c32ddf9), the ledger row update was premature because the PR had not merged yet and MERGE_COMMIT field was empty. Ledger updates must be added in a separate commit after merge with the actual merge commit SHA. Reverting docs/spec/implementation_status.csv to master state and regenerating docs/spec/IMPLEMENTATION_STATUS.md accordingly. Code changes (validation.ts, validation.test.ts) remain intact. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
AUTHOR rework: Address ACCEPTOR REQUEST_CHANGESBranch: Changes madePer ACCEPTOR feedback (zendev-acceptor REQUEST_CHANGES at c32ddf9), removed the premature ledger row update from this PR. The ledger must be updated only after merge with the actual merge commit SHA, not inside the PR. Reverted:
Retained:
VerificationAll checks remain green: TypeScript (canonical):
C# Legacy (.NET 9):
PR body updatedThe "Changed artifacts" section in the PR body now correctly lists only the source code changes, and ledger-promotion references have been removed. HandoffPull request #300 ready for ACCEPTOR re-review. All acceptance criteria met, all checks passed, no blocking gates remain. |
There was a problem hiding this comment.
ACCEPT — REQ-CONFIG-003 extraction coupling validation at revision a23d255
Verification summary
- ✅ All required checks green at head revision: build-and-test, policy-guard, typescript, mergeability
- ✅ Independent verification completed:
- TypeScript: typecheck/test/build all passed (442 tests)
- C# Legacy: build/test all passed (45 tests)
- ✅ All 6 acceptance criteria met:
- Resource-without-amount validation present and tested
- Amount-without-resource validation present and tested
- Valid recipes with both fields pass validation
- Existing numeric validation remains intact
- All TypeScript and C# tests remain green
- Ledger update correctly removed (post-merge reconciliation)
- ✅ Diff confined to declared scope (validation.ts and test changes only)
- ✅ Handoff record complete with all required sections
- ✅ No secrets, credentials, or test weakening
The AUTHOR correctly addressed the prior REQUEST_CHANGES by removing the premature ledger update. The coupling validation is properly implemented as the smallest reversible change satisfying all acceptance criteria.
…rged (#304) Update REQ-CONFIG-003 from PARTIAL to IMPLEMENTED after PR #300 (Issue #299) merged the missing extraction resource/amount coupling validation. The SLOPSTER QA finding about extraction recipes omitting required amounts has been fixed by validateDefinitionPack now enforcing section 16A coupling constraint. Promote REQ-CONFIG-004 from PARTIAL to IMPLEMENTED now that its recorded CONFIG-003 blocker has cleared. Re-evaluate confirms the two slices (reconcileGenesisStocks and BOND_OPENING_POSITION exclusion) implemented in PR #208 satisfy the full requirement with 11 passing tests and both runtime suites green. Changes: - REQ-CONFIG-003: IMPLEMENTED (was PARTIAL), PR #300 merge 91507ac - REQ-CONFIG-004: IMPLEMENTED (was PARTIAL), keeps PR #208 but removes obsolete CONFIG-003 blocker text from evidence Both rows now reflect the resolved state with accurate evidence linking the merged pull requests that contributed to each requirement. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Update REQ-CONFIG-003 ledger ISSUE field from 167 to 299 to correctly reference the completing repair (Issue #299, PR #300) rather than the historical issue #167. Regenerate IMPLEMENTATION_STATUS.md from the corrected CSV to maintain derived-document consistency. Acceptance criteria from Issue #307: - [x] REQ-CONFIG-003 has structured ISSUE=299, PR=300, MERGE_COMMIT=91507ac - [x] Historical contributing work remains in EVIDENCE - [x] IMPLEMENTATION_STATUS.md regenerated showing Issue #299 - [x] implementation_status.py --check passes - [x] CSV parses as exactly six columns Verification: - npm ci: 0 vulnerabilities - npm run typecheck: passed - npm test: 472 tests passed - npm run build: succeeded - dotnet build: succeeded - dotnet test: passed Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
2 participants
Summary
This pull request enforces the REQ-CONFIG-003 section 16A validation rule for extraction recipe coupling: a RecipeDefinition must not name an extracted resource without a positive extracted amount, and must not carry a positive extracted amount without the corresponding resource identity.
Issue and tested revision
Closes #299
Tested at revision: HEAD of claude/issue-299-extraction-coupling
Changed artifacts
Acceptance criteria status
rejects extractionResourceId without extractedResourcePerBatchrejects extractedResourcePerBatch without extractionResourceIdaccepts a valid extraction recipe with both extractionResourceId and extractedResourcePerBatchrejects extractedResourcePerBatch <= 0now includes valid extractionResourceIdCheck results
TypeScript (canonical)
C# Legacy (.NET 9)
Verification method
Assumptions and unknowns
None. Section 16A validation rule is clear and was previously documented in the issue description. Coupling validation is the smallest reversible change that satisfies acceptance criteria.
Highest-risk area for review
Validation order: coupling check runs before positive-amount check, so if an extraction recipe has both extractionResourceId and a zero/negative extractedResourcePerBatch, it will fail on the positive-amount check (which is correct behavior). This ordering ensures the most specific error message is shown.
Blocking gates
None. All acceptance criteria satisfied, all checks passed. Ready for ACCEPTOR review.
Co-Authored-By: ACCEPTOR feedback integration noreply@github.com