Repository navigation
Conversation
Record where a fix has to land, and what counts as a regression or as evidence, in the invariants guide. AGENTS.md keeps a single pointer. Co-authored-by: Ragnor Comerford <ragnorc@users.noreply.github.com>
WhyReviews kept restating where a fix must land, and what counts as a regression or as evidence. The repository already states the five-change test, one source of truth, and the query-test tiers. It did not state those two rules. WhatThis pull request adds the section "Root cause and change discipline" to HowThe section cites hard invariants 1, 12, and 13. It cites the governing principle. It cites the plan-replay tiers and the Lance compatibility gates. It does not copy those texts. ImpactAn author can check one section before review. A reviewer can cite that section. One paragraph conflicts with the composition rule in the systems guide, and with the publication and policy contracts. Change that paragraph before merge. The other rules reduce long-term liability. They add one citation target, and they do not add a second copy of an existing rule. Recommendation: revise. Blocking
A reader can take that sentence in a narrow way. Do not re-prove a primitive's own guarantee at every call site. The words say more than that. They say the combination inherits correctness, efficiency, and security. A primitive that owns one guarantee does not give the combination those three properties. That conflicts with The same claim conflicts with the graph contract in code. Hard invariant 2 says a per-table commit is not graph publication. The ratio in the same paragraph is not a check a reviewer can perform. It puts correctness, efficiency, and security in one numerator. Keep the preference for a small set of primitives, each of which owns one guarantee. State that a combination still has to discharge the composed contract. Remove the ratio. The checklist question at lines 268-269 can remain. It does not say the combination skips the proof. Optional
Tradeoffs and liabilityThe section belongs in the invariants guide. That page already holds the standing rules for a person who proposes, implements, or reviews a change. The systems guide remains the evaluation essay. It is not an authoring manual. The regression rule and the evidence rule are new. They sit with the design rule so a review cites one section. They link the testing guide. They do not copy the tier ladder or hard invariants 1, 12, and 13. A copy would drift. Moving only the evidence sentences into the testing guide would split one citation across two pages. The single section has lower long-term liability.
No changelog fragment is correct. The documentation guide requires a note for a user-visible change. These are contributor rules. The title type is I read the open pull requests as context only. They are not requirements for this change. Group commit (pull request 785) and the open permission RFCs still argue a composed publication and an explicit grant at the boundary. That matches the blocking finding. It does not add a requirement. This pull request changes no behavior. A GQT case or a Rust regression test does not apply. ValidationReviewed commit Local results:
Exact-commit CI, run 41253471096, head
Code and document inspection, separate from those commands: hard invariants 1, 12, and 13 match the citations. Plan replay states the two query tiers. Lance compatibility keeps workspace, GQT, Clippy, and object-store results as separate evidence. LimitationsI did not download upstream Lance pages. The diff states no new substrate behavior. I read |
WhyThe review on WhatThe new paragraph prefers orthogonal primitives. It uses Eric Raymond's definition from The Art of Unix Programming. It says a composed operation still has to establish its own contract. It cites section 6.1 of the systems guide. The guarantee-per-code aim is now a direction, not a number. The same commit applies the three optional notes from that review. Earlier findingsBlocking finding: fixed. The paragraph no longer says a combination inherits correctness, efficiency, and security. It no longer says the combination can skip the proof. It says composition does not discharge obligations. Orthogonality makes that argument shorter. It does not make the argument optional. That matches section 6.1 of The Raymond sentence matches the page it links. In a purely orthogonal design, each operation changes one property. It has no side effect on other properties. There is one way to change each property. The page has the anchor The ratio is no longer a check. The text calls the aim a direction for judgment. It says the checklist is how a review applies that aim. The checklist does not trade correctness for less code. A new burden is still justified against the governing principle (lines 216-217). Optional 1: taken. Line 207 says "parallel machinery". Those are the words of hard invariant 1. Optional 2: taken. Lines 252-254 use the testing guide's list: rows, counts, result column types, or errors. They tell the author to say why GQT cannot express the contract. Optional 3: taken in the form that note asked for. Line 286 asks for the callers and workloads that share the cause. A yes answer is that list. It does not ask for proof that the list is complete. The old question that joined two cases with "or" is gone. The scope of the replacement question is a new optional note below. ImpactThe conflict with section 6.1 is gone. A reviewer can cite this section. Two optional notes are new. They do not restore the inheritance claim. This commit adds no behavior and no second copy of an existing rule. Recommendation: ready. BlockingNone. Optional
Section 6.1 requires the composition to discharge its assumptions, align representations, preserve publication and failure semantics, and establish progress for the whole path. It does not require a measured cost. It does not put engine policy on every composition. Hard invariant 2 is the one door for a graph-content publication. A query does not publish. Hard invariant 10 is the trust boundary. Hard invariant 13 requires a test at the layer whose contract changed. A cost claim needs a checked-in instrument. Lines 256-259 already say that. A composed operation with no cost claim does not need a measured cost. Keep the sentence that says composition does not discharge obligations. Name each example in the scope its own invariant gives.
Tradeoffs and liabilityThis commit does not move the section. The invariants page remains the authoring rule. The systems guide remains the evaluation essay. Orthogonality has one owner, this section, with a link for Raymond's definition. Composition obligations stay owned by section 6.1. The new text links that section. It does not copy the four-role table. A copy would drift. The formal-core paragraph matches hard invariants 7 and 9. A fast path may change cost. It must keep the same observable contract, and a test must compare it with the general path. The paragraph does not ban a heuristic. No new code type or module is warranted. The new concept is a rule a review can cite. This section is the owner. The link to section 6.1 is the source of truth for composition. The optional note above is the place that gloss can drift. The removed liability is the inheritance sentence. A reviewer can no longer cite this section to skip the composed contract. The added liability is two sentences that name a wider scope than the invariants they cite. Five more paragraphs of the same kind would turn this page into a second systems essay. This commit does not do that. It stays in one section and links the existing owners. Open pull requests are context only, as lines 262-264 say. Pull request 785 (group commit) and the open permission RFCs still argue one publication and an explicit grant at the boundary. That agrees with the fixed paragraph. It adds no requirement for this change. The diff of this commit is
ValidationReviewed commit Local results, isolated checkout of that commit:
Exact-commit CI, head
Code and document inspection, separate from those commands: section 6.1, hard invariants 1, 2, 7, 9, 10, and 13, the plan-replay tiers, No regression command applies. LimitationsI did not fetch the Lance format pages. This commit states no substrate behavior. I read |
Reviewers were carrying a long prompt of principles that the repository already states: the five-more-changes test, one source of truth, correctness before simplicity before performance, checking a Lance claim against the upstream code, the observable-contract framing, and reproducing a predicted failure before fixing it. What the repository did not state is the root-cause and design-hygiene bar. Review after review flagged the same defects: a patch that treats the symptom, a special case at the wrong layer, a new mechanism beside an abstraction that already gave the guarantee, duplicated state that can drift, reliance on another component's undocumented internals, a "regression" that never failed for the predicted reason (a compile error, a setup failure, a skipped test, or a run that executed nothing), and a write-up that treats a local run, CI, and a reading of the code as the same evidence. Open pull requests, documented direction, and future Lance versions were also being treated as if they had already shipped. These are rules for the person or agent writing the change, so they belong in the repository. Authors can catch them before review, and a reviewer can point at one section instead of re-arguing the point.
The review workflow stays out. Author eligibility, one review per run, posting and dedupe, the plain-English style of review text, and notifying Ragnor are his tooling, not rules for someone changing the code.
The rules are one new section, Root cause and change discipline, in docs/dev/invariants.md. That page already owns standing design rules for anyone proposing, implementing, or reviewing a change. docs/dev/systems.md stays the evaluation essay; it is not an authoring manual. Where a rule was already present, the section links it instead of copying it: hard invariants 1 and 12 for substrate machinery and a shadow copy that can drift, the governing principle for the liability test behind "fewer lines is not proof," hard invariant 13 for a cost claim's instrument, Lance compatibility in the testing guide for keeping storage gates separate, and the query-behavior tiers so a shallow .gqt case is not a substitute for saying why GQT cannot express the contract. The review checklist at the end of the same page asks those questions in the checklist's existing shape. AGENTS.md stays a map: Change discipline has one pointer line to the section, next to the existing "fix the root cause" bullet. docs/dev/index.md names change discipline on the invariants row so the developer guide's map matches the page. There is no changelog note. The documentation guide asks for one only on a user-visible change, and a docs title does not require one.
python3 scripts/check-docs.py printed Documentation OK (441 Markdown files checked) and exited 0. bash scripts/check-agents-md.sh printed AGENTS.md ↔ docs indexes OK (66 links, 62 docs); engine reads no setting variable, and exited 0. python3 scripts/check-pr-title.py --title "docs: add root-cause and change-discipline rules" printed PASS PR title accepted (48/100 characters) and exited 0. typos 1.50.1, the version ci.yml pins, on AGENTS.md, docs/dev/invariants.md, and docs/dev/index.md exited 0 with no findings.
For review: the decision to challenge is the home. The evidence sentences could have gone into the testing guide; they sit in the invariants guide so one section is the rule a review cites, and they link the testing page instead of copying the tier ladder or hard invariants 1, 12, and 13. The checklist questions restate the section the way the existing checklist restates the hard invariants. What would make that wrong is a second copy of the tier ladder, or treating open pull requests and future Lance versions as requirements the section had meant only to name as context.