Require a Tracking Value on an investigate Divergence Disposition - #1556
Conversation
`investigate` is the one disposition in the divergence ledger's vocabulary that names no outcome. The other five each state what happens to the divergence, where this one states only that nobody has decided yet, so an entry carrying it beside a null `tracking` recorded that a decision was owed while naming nowhere it was being made, and the deferral then read as an omission. Make the requirement structural rather than a one-time ledger edit. The schema gains a conditional requiring a non-empty `tracking` on an `investigate` entry, in the `dispositions` and `gaps` arrays alike, and `spec/validate.py` gains the same check so CI refuses one rather than an editor alone. The field stays optional under every other disposition, each of which already names its own outcome, and a test case holds that scoping. The ledger's own note records the asymmetry beside the vocabulary it belongs to, and `README.md` records it beside the entry shape. The schema and the gate are deliberately unequal, and in one direction only. The schema refuses an absent, null, empty, or non-string value, and the gate refuses every one of those plus a value that is only whitespace. Closing that last gap with a schema pattern would refuse a value the gate accepts, which is the direction #1504 reverted for the registry schema, for this same reason: U+FEFF is an ECMA-262 `\s` character and `str.strip` does not remove it, so a `\S` pattern would refuse a value this gate keeps. A mirror test pins both halves and that direction, and a second test runs the real script against a scratch ledger deferring in both arrays, since a test of the helper alone stays green when either call site is deleted. The ledger's one `investigate` row now tracks #1553, which holds the outstanding `pyproject.toml` question, rather than #669. Pointing it at #669 while closing #669 as fixed would leave the only governed row naming a closed issue where nothing is being decided, which is the state #669 was filed about, with both new gates reading clean over it. Refs #669 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe divergence ledger schema and validator now require non-empty tracking values for ChangesDivergence tracking enforcement
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The tracking requirement is consistently documented and enforced for investigate entries, with coverage for both dispositions and gaps. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 2 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🟢 Approval recommended
The schema, validator, tests, and documentation changes are consistent and collectively enforce the new investigate tracking invariant without impacting other dispositions.
Pull request overview
This pull request makes tracking structurally required for divergence-ledger entries whose disposition is investigate, ensuring a deferred decision always names where that decision is being made (schema + validator enforcement), and updates the ledger/docs to reflect the new invariant.
Changes:
- Add an
investigate_tracking_errorsvalidator helper and wire it into both thedispositionsandgapsloops inspec/validate.py. - Extend
spec/divergences.schema.jsonwith a shared$defsconditional (investigateNeedsTracking) to require a non-emptytrackingstring whendisposition == "investigate". - Update the live ledger entry and README prose to document/reflect the
investigate-only requirement.
File summaries
| File | Description |
|---|---|
| spec/validate.py | Adds and wires a validator helper enforcing non-empty tracking for investigate dispositions. |
| spec/divergences.schema.json | Adds a $defs conditional and applies it to both ledger arrays to require tracking for investigate. |
| spec/divergences.json | Updates the remaining investigate row’s tracking to the successor issue and documents the new asymmetry in the note. |
| scripts/tests/test_spec_validate.py | Adds unit + integration-style tests proving the helper behavior and its wiring into both loops, plus schema mirror assertions. |
| README.md | Updates the divergence-ledger description to document the investigate-only tracking requirement. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Declining the Docstring Coverage pre-merge warning (27.78 percent against a required 80 percent), with evidence rather than an assertion, and filing the class question rather than re-arguing it per pull request. Why it does not apply here:
What this branch actually added, so the number is not left to speak for itself: all three new test classes carry a class docstring, the new Stated plainly rather than buried: at 23.1 percent the methods added here do sit below that 49.6 percent baseline, so this is a deliberate style judgment about self-describing test names and not a claim that the additions match the file's rate. The class question, whether this check should be disabled the way markdownlint, actionlint, shellcheck and ruff already are, or given a threshold that matches the fleet's real convention, is filed as #1558. |
) Promotes `develop` to `main`, carrying one change. ## Require a Tracking Value on an investigate Divergence Disposition (#1556) `investigate` is the one disposition in the divergence ledger's vocabulary that names no outcome. The other five each state what happens to the divergence, where this one states only that nobody has decided yet, so an entry carrying it beside a null `tracking` recorded that a decision was owed while naming nowhere it was being made, and the deferral read as an omission. That requirement is now structural: a `$defs` conditional in `spec/divergences.schema.json` applied to both arrays, the same check in `spec/validate.py` so CI refuses such an entry rather than an editor alone, and the asymmetry recorded in the ledger's own note and in `README.md` beside the entry shape. The field stays optional under every other disposition, each of which already names its own outcome, and a test holds that scoping. The schema and the gate are deliberately unequal in one direction only, the gate additionally refusing a whitespace-only value, because a schema pattern closing that gap would refuse a value the gate accepts, which is the direction #1504 reverted for the registry schema. ## What this does not settle The ledger's one `investigate` row tracks #1553 rather than #669, which is why closing #669 here is honest: the rule it asked for has shipped, and the decision that row defers has its own issue to be decided in. Three follow-ups stay open deliberately, none of them blocking this promotion: - #1553, whether the Python repos carry an equivalent config-only `pyproject.toml`. Its measurement half is answered on the issue: all three carriers hold one, none holds an equivalent, and tool placement is not a fleet constant. What remains is the fleet-wide new-findings tradeoff, which is the maintainer's call. - #1554, `reports/divergences.md` lagging the ledger's tracking values in two rows. It is generated rather than hand-edited and regenerating is a networked run, so it lands on its own where the fleet drift it reports is what is being read. - #1555, `TODO.md`'s Stage 4 note calling `validate-task.yml` an `investigate` gaps entry where the ledger dispositions it `retire`. Pre-existing, and it predates this change's merge base. ## Verification Four local adversarial passes ran over this change, the last finding nothing in any category. Between them they corrected a false citation twice, moved the governed ledger row off the issue being closed, and added a wiring test after a pass showed that deleting either `main()` call site kept every test green. Five mutants are killed by the current tests, including one at each call site. A real `jsonschema` probe over 192 entry shapes confirms the schema is stricter than the gate nowhere. On #1556: every CI job passed and Copilot reviewed the full head with no findings. Closes #669 🤖 Generated with [Claude Code](https://claude.com/claude-code)
What this changes
investigateis the one disposition in the divergence ledger's vocabulary that names no outcome. The other five each state what happens to the divergence, where this one states only that nobody has decided yet, so an entry carrying it beside a nulltrackingrecorded that a decision was owed while naming nowhere it was being made, and the deferral read as an omission.This makes the requirement structural rather than a one-time ledger edit:
spec/divergences.schema.jsongains a$defsconditional requiring a non-emptytrackingon aninvestigateentry, applied to thedispositionsandgapsarrays alike, and bothtrackingdeclarations point at it.spec/validate.pygains the same check, as a pureinvestigate_tracking_errorshelper wired into both loops, so CI refuses such an entry rather than an editor alone.spec/divergences.json's own note andREADME.md's ledger sentence both record the asymmetry beside the shape they describe.Why a ledger edit would not have closed #669
The issue names three entries. Two left the
investigateset already, settled in #736, and the third,pyproject.toml, already carried atrackingvalue. Re-applying that edit would have closed nothing: the issue's actual subject, thatinvestigateschedules no decision and records no owner, regenerates the moment anyone writes the next entry. Hence the rule rather than the row.The one row moved to a successor issue
That row previously tracked #669 itself. Closing #669 as fixed by this change would have left the fleet's only governed row pointing at a closed issue where nothing was being decided, which is the state #669 was filed about, with both new gates reading clean over it. The still-owed decision, whether the Python repos carry an equivalent config-only
pyproject.toml, is now #1553, and the row'strackingand its reason point there.The schema and the gate are deliberately unequal, in one direction
The schema refuses an absent, null, empty, or non-string value. The gate refuses every one of those plus a value that is only whitespace. Closing that last gap with a schema
patternwould refuse a value the gate accepts, which is the direction #1504 reverted for the registry schema, for this same reason: U+FEFF is an ECMA-262\scharacter andstr.stripdoes not remove it, so a\Spattern would refuse a value this gate keeps. So the schema carries adescriptionnaming the gate's extra rule, andInvestigateTrackingSchemaMirrorCasepins both halves and that direction, in the shapeRegistrySchemaMirrorCasealready established for the advisory registry schema. Verified with a real validator: the schema is nowhere stricter than the gate.Verification
jsonschema4.26.0, reached throughuvxsince it is not installed: the live ledger validates clean, and null, absent, and emptytrackingare each rejected. Switching the disposition toacceptedwith nulltrackingvalidates clean, so the requirement does not leak onto the five dispositions that name an outcome.InvestigateTrackingWiringCaseruns the real script against a scratch ledger deferring in both arrays, with a marker defect per array proving each loop was reached. Mutation-checked: deleting either call site, swapping thedispositionslabel to thegapform, weakening the.strip(), removing thegapsconditional, and relaxingminLengthto0each fail the new tests.ruff format --check,ruff check,mypy,spec/validate.py, 1400 self-tests,spec/audit.py --selftest,spec/workflow_reuse.py --selftest, the prose gate whole-tree, the EOL check, andbuild_dist --check.canonical_review.pyreports no carried canonical unit changed.Three findings not fixed here, with reasons
TODOsatisfies it. The maintainer's decision was that aninvestigateentry gains a tracking value, and a strict issue-shaped pattern would reject legitimate tracker forms, so the error message names the expected form and the second legitimate remedy instead: where no decision is pending, the fix is the disposition that names the outcome.tracking, such as42, reports two lines, the pre-existing type error and this one. Both are true and neither shadows the other, so this is report noise rather than a wrong answer.reports/divergences.mdstill renders#669for that row. It is regenerated rather than hand-edited, and regenerating joins against live fleet reality over the network, so it would pull unrelated fleet drift into this diff. That report already lagged the ledger independently, on the.markdownlint-cli2.jsoncrow sincea37e8a8. Both rows are tracked in Regenerate reports/divergences.md, whose tracking values lag the ledger in two rows #1554.Refs #669
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Validation