Conversation
) ## What this changes `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. This makes the requirement structural rather than a one-time ledger edit: - `spec/divergences.schema.json` gains a `$defs` conditional requiring a non-empty `tracking` on an `investigate` entry, applied to the `dispositions` and `gaps` arrays alike, and both `tracking` declarations point at it. - `spec/validate.py` gains the same check, as a pure `investigate_tracking_errors` helper wired into both loops, so CI refuses such an entry 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. - `spec/divergences.json`'s own note and `README.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 `investigate` set already, settled in #736, and the third, `pyproject.toml`, already carried a `tracking` value. Re-applying that edit would have closed nothing: the issue's actual subject, that `investigate` schedules 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's `tracking` and 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 `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. So the schema carries a `description` naming the gate's extra rule, and `InvestigateTrackingSchemaMirrorCase` pins both halves and that direction, in the shape `RegistrySchemaMirrorCase` already established for the advisory registry schema. Verified with a real validator: the schema is nowhere stricter than the gate. ## Verification - The schema conditional is machine-checked under `jsonschema` 4.26.0, reached through `uvx` since it is not installed: the live ledger validates clean, and null, absent, and empty `tracking` are each rejected. Switching the disposition to `accepted` with null `tracking` validates clean, so the requirement does not leak onto the five dispositions that name an outcome. - The validator wiring is proved by a positive control rather than by a green run, since a compliant ledger only exercises the accept path. Nulling the tracking value in a scratch tree produced exactly the new error, 20 errors against the baseline's 19, and that message appears nowhere in the baseline output. - `InvestigateTrackingWiringCase` runs 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 the `dispositions` label to the `gap` form, weakening the `.strip()`, removing the `gaps` conditional, and relaxing `minLength` to `0` each fail the new tests. - Gates green on the final content: `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, and `build_dist --check`. `canonical_review.py` reports no carried canonical unit changed. ## Three findings not fixed here, with reasons - **The check is presence-only**, so a string such as `TODO` satisfies it. The maintainer's decision was that an `investigate` entry 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. - **A non-string truthy `tracking`, such as `42`, 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. - **The generated `reports/divergences.md` still renders `#669` for 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.jsonc` row since `a37e8a8`. Both rows are tracked in #1554. Refs #669 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Clarified that entries marked **investigate** must include a tracking value identifying where the decision is being made. - Documented that this requirement applies to both dispositions and gaps. - **Validation** - Added validation to reject investigate entries with missing, empty, or whitespace-only tracking values. - Updated the schema to conditionally require tracking information for investigate entries while preserving existing tracking type rules. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 44 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
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 |
There was a problem hiding this comment.
🟢 Approval recommended
The change is consistently implemented across schema, validator, tests, and documentation, with coverage proving both ledger loops enforce the new rule.
Pull request overview
This promotion carries the change that makes tracking required when a divergence entry uses the investigate disposition, ensuring the ledger records where the pending decision is being made (instead of reading like an omission).
Changes:
- Add a validator check in
spec/validate.pythat rejectsinvestigateentries with missing/empty/whitespace-onlytracking, applied to bothdispositionsandgaps. - Tighten the advisory JSON schema with a conditional that requires a non-empty string
trackingwhendisposition == "investigate". - Update the divergence ledger note and README documentation to record the asymmetry (required for
investigate, optional otherwise), and adjust the one existinginvestigaterow to point at the successor tracking issue.
File summaries
| File | Description |
|---|---|
| spec/validate.py | Adds investigate_tracking_errors() and wires it into both divergence-ledger loops. |
| spec/divergences.schema.json | Adds $defs/investigateNeedsTracking and applies it to both arrays via allOf. |
| spec/divergences.json | Documents the rule in the ledger note and updates the pyproject.toml row tracking value to #1553. |
| scripts/tests/test_spec_validate.py | Adds unit tests for the helper, wiring tests exercising both loops, and a schema-mirror test asserting the intended schema/gate relationship. |
| README.md | Updates the divergence-ledger documentation to state 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.
|
@coderabbitai review |
|
|
Recording CodeRabbit's review coverage on this promotion, since the bot's own comment above says "Review rate limited" and a later reader would otherwise have to re-derive whether coverage is missing. CodeRabbit did not review this head. It is rate limited here, and its notice adds that it "does not re-review already reviewed commits". No retry window is given. The content it carries was already reviewed, and that is checkable rather than asserted. The squash commit this PR promotes and the head CodeRabbit reviewed on #1556 have the identical tree: Only the commit differs, by its parent. On #1556 CodeRabbit named its scope as "between 4486984 and 4c31860" and reported no actionable comments for that tree. So re-prompting an incremental reviewer here would return exactly the notice above rather than a second reading, which is why this is recorded instead of retried. Copilot did review this head, 5 of 5 changed files, 0 comments generated, approval recommended. One item from #1556 carried over rather than reopened: the Docstring Coverage pre-merge warning was declined there with evidence, no CI step, hook, or ruff |
Promotes
developtomain, carrying one change.Require a Tracking Value on an investigate Divergence Disposition (#1556)
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. That requirement is now structural: a$defsconditional inspec/divergences.schema.jsonapplied to both arrays, the same check inspec/validate.pyso CI refuses such an entry rather than an editor alone, and the asymmetry recorded in the ledger's own note and inREADME.mdbeside 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
investigaterow 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: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.reports/divergences.mdlagging 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.TODO.md's Stage 4 note callingvalidate-task.ymlaninvestigategaps entry where the ledger dispositions itretire. 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 realjsonschemaprobe 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