refactor(contracts): prove the hand-mirrored contract halves agree#31
Closed
sturlese wants to merge 1 commit into
Closed
refactor(contracts): prove the hand-mirrored contract halves agree#31sturlese wants to merge 1 commit into
sturlese wants to merge 1 commit into
Conversation
ADR 001/011 doctrine: packages share no code and talk through files, so
every cross-package contract exists as hand-mirrored copies. The
architecture review found three of those copies drifting (one had
already produced the empty-ACL leak fixed in the previous commit).
This closes the remaining drifts and makes CI prove the agreement:
- factstore.query_facts now carries the same semantics as the serving
layer's copy (answer/metrics.query_metrics): verified-only rows and
case-folded metric/entity inputs. The store only ever holds verified
lowercase rows, so this makes invariants explicit rather than
changing results — but until now the golden evals exercised the
pipeline copy while production served through the answer copy, with
nothing guarding the gap.
- graph merges.build_merge_judge honored only half the CLEAN_MODEL
contract: a provider-prefixed value ("anthropic:claude-sonnet-4-5")
worked in clean but was fed into OpenAIResponsesModel here and broke;
a CLEAN_LLM typo silently picked the fake (startswith). Now a
faithful mirror of clean's llm.build_model (provider passthrough,
validated effort) with fail-fast backend validation, tested.
- answer's ANSWER_LLM dispatch gets the same fail-fast (a typo fell
through to the fake or the real path); dead PAGE_EXCERPT copy in
synthesize.py removed; the raw superseded-pages SQL inlined in the
service orchestrator moved behind index.superseded_paths — the page
index owns its queries.
- clean now has ONE frontmatter-block definition (page.FRONTMATTER_RE):
versions.annotate_page and claims.strip_page_chrome parsed the same
block with two hand-rolled regex variants.
- clean/state.py documents the state schema in the one module every
consumer imports — five modules mutate this dict in place and its
shape was recorded nowhere.
- evals: new eval_contract_parity proves, in the one harness that
imports both sides, that (a) clean's visible(list) and answer's
visible(csv) agree over the full truth table including the
empty-intersection case, and (b) query_facts and query_metrics return
identical rows for the same probes over the produced store. Scorecard
goes 22 -> 24 metrics (README updated).
Validation: clean 248, graph 38, answer 42, fetch 29, corpus 48,
slack 13, benchmark 3 (bare pytest, CI mode); ruff clean; golden
scorecard 24/24.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #30. ADR 001/011 doctrine means every cross-package contract exists as hand-mirrored copies; the architecture review found three of them drifting (one had already produced the empty-ACL leak fixed in #30). This PR closes the remaining drifts and makes CI prove the agreement instead of trusting the mirror comments.
Changes
clean/factstore.query_factsnow carries the same semantics as the serving copy (answer/metrics.query_metrics): verified-only rows, case-folded metric/entity inputs. No result changes today (the store only holds verified lowercase rows) — but the golden evals exercised the pipeline copy while production served through the answer copy, with nothing guarding the gap.merges.build_merge_judgefed a provider-prefixedCLEAN_MODEL(anthropic:claude-sonnet-4-5) intoOpenAIResponsesModel(broken, while clean supported it) and aCLEAN_LLMtypo silently picked the fake (startswith). Now a faithful hand-mirror of clean'sllm.build_model(provider passthrough, validated reasoning effort) plus fail-fast backend validation — tested.answer/synthesize.build_synthesizer(a typo fell through silently). Also: deadPAGE_EXCERPTduplicate removed, and the raw superseded-pages SQL inlined in the service orchestrator moved behindindex.superseded_paths()— the page index owns its queries.page.FRONTMATTER_RE) —versions.annotate_pageandclaims.strip_page_chromeeach had their own regex variant of it.state.py, the one module every consumer imports — five modules mutate this dict in place and its shape was recorded nowhere.eval_contract_parityproves in CI (the one harness that imports both sides) that clean'svisible(list)and answer'svisible(csv)agree over the full truth table — including the empty-intersection case fix(acl): an empty ACL was served open; forbid labels that break CSV round-trip #30 fixes — and thatquery_facts/query_metricsreturn identical rows for the same probes over the produced store. Scorecard: 22 → 24 metrics (README updated).Testing
pytest(CI mode).Notes
Deliberately NOT touched, with reasons: shared contract library (contradicts ADR 001/011 — the parity eval is the doctrine-compliant guard), typed state objects (ADR 001 defers them; the schema is now documented instead),
slugifycap alignment clean↔graph (changes existing corpus paths — migration-sensitive), and the evals/benchmark harness merge (the golden gate is tuned; not worth the risk in this pass).