From fd80da6ac0e1933cfc493fe064e70df03960df27 Mon Sep 17 00:00:00 2001 From: Shay Date: Mon, 18 May 2026 13:36:37 -0700 Subject: [PATCH] docs(adr-0058): forward_graph_constraint engaged-but-inert; null-lift pinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0058 closes the ADR-0047 follow-up question ("should the forward_graph_constraint flag become default-on or pack-opt-in?") with the explicit answer: neither, yet. The ADR-0047 A/B characterisation found that the flag is observably inert on every public-cognition-lane metric — narrowing which tokens the walk may visit did not change which surface gets emitted. That finding scoped ADR-0048..0053, which closed the cognition lane to 100.0% surface_groundedness / 91.7% term_capture_rate via realizer / surface-assembly work downstream of propagation. This ADR makes three load-bearing decisions: 1. `forward_graph_constraint` remains opt-in with default `False`. No identity pack (including precision_first_v1) opts in. 2. No `runtime_preferences` block is added to identity packs; no path from pack JSON to RuntimeConfig is opened. Deferring the pack-to-runtime composition layer until at least one such preference has demonstrated lift avoids letting the wiring lead the lift and locking in an abstraction at the wrong level. 3. The ADR-0047 null-lift finding is promoted from a historical observation to a CI-enforced invariant. A new regression test runs the public cognition split twice (flag OFF vs ON) and asserts every watched metric is pair-wise identical. If downstream realizer work later moves a metric on the flag flip, the test fails as a deliberate transition rather than silent drift. - docs/decisions/ADR-0058-forward-graph-constraint-status.md — ADR doc. - docs/decisions/README.md — index entry. - tests/test_forward_graph_constraint_null_lift.py — 2 tests: null-lift invariant across the cognition lane, default-False contract. Verification: smoke 67 passed; flag tests 7 passed (5 wiring + 2 null-lift). No runtime behaviour change; versor_condition < 1e-6 invariant unaffected. --- ...DR-0058-forward-graph-constraint-status.md | 153 ++++++++++++++++++ docs/decisions/README.md | 1 + ...test_forward_graph_constraint_null_lift.py | 73 +++++++++ 3 files changed, 227 insertions(+) create mode 100644 docs/decisions/ADR-0058-forward-graph-constraint-status.md create mode 100644 tests/test_forward_graph_constraint_null_lift.py diff --git a/docs/decisions/ADR-0058-forward-graph-constraint-status.md b/docs/decisions/ADR-0058-forward-graph-constraint-status.md new file mode 100644 index 00000000..8e03d22f --- /dev/null +++ b/docs/decisions/ADR-0058-forward-graph-constraint-status.md @@ -0,0 +1,153 @@ +# ADR-0058 — `forward_graph_constraint`: Engaged but Inert on Today's Cognition Lane + +**Status:** Accepted +**Date:** 2026-05-18 +**Author:** Shay + +--- + +## Context + +[ADR-0046](./ADR-0046-forward-graph-constraint.md) introduced +`build_graph_constraint` — the function that converts a +`PropositionGraph` into an `AdmissibilityRegion` *before* `generate()` +runs. [ADR-0047](./ADR-0047-wire-forward-graph-constraint.md) wired +it into the chat hot path behind an opt-in +`RuntimeConfig.forward_graph_constraint` flag (default `False`) and +ran the A/B characterisation on the 13-case public cognition split: + +| Metric | Flag OFF | Flag ON | Δ | +|----------------------------------------|----------|---------|---| +| `intent_accuracy` | 100.0 % | 100.0 % | 0 | +| `surface_groundedness` | 15.4 % | 15.4 % | 0 | +| `term_capture_rate` | 0.0 % | 0.0 % | 0 | +| `versor_closure_rate` | 100.0 % | 100.0 % | 0 | +| Cases producing non-trivial constraint | n/a | 6 / 13 | — | + +The architectural finding: **narrowing which tokens the walk may visit +did not change which surface gets emitted on this lane.** The +surface-grounding gap lived downstream of propagation. That scoping +drove the next four ADRs — [0048](./ADR-0048-pack-grounded-surface.md) +through [0053](./ADR-0053-cognition-lane-closure.md) — which closed +the cognition public split at **100.0% surface_groundedness / 91.7% +term_capture_rate** by working in the realizer / surface-assembly +layer, not in propagation. + +This ADR closes the loop on ADR-0047's open question by **deciding +not to flip the flag's default**, and by promoting the null-lift +observation from a historical A/B reading to a regression-tested +invariant. + +--- + +## Decision + +1. **`forward_graph_constraint` remains opt-in with default `False`.** + No production identity pack — including `precision_first_v1` + (which would be the natural home for a precision-narrowing + structural constraint) — opts into the flag. + +2. **No `runtime_preferences` block is added to identity packs.** No + path is opened today for identity packs to override + `RuntimeConfig` fields. Such a path would require: + + - A new schema field in pack JSON. + - Re-ratification of every identity pack (mastery seal). + - Loader work to expose runtime preferences. + - `ChatRuntime` composition work to merge pack prefs into the + active `RuntimeConfig` at boot. + + Doing that infrastructure for an `engaged_but_inert` feature + would let the wiring lead the lift — and lock in + pack-to-runtime coupling at an abstraction level we may regret + when the constraint *does* matter downstream. + +3. **The ADR-0047 null-lift finding becomes a CI-enforced invariant.** + `tests/test_forward_graph_constraint_null_lift.py` runs the full + cognition public split twice — flag OFF vs ON — and asserts every + watched metric is pair-wise identical. If a downstream change in + the realizer / surface-assembly layer ever moves a metric on the + flag flip, this test fails and the architectural assumption that + the constraint is observably inert on this lane gets re-examined + as a deliberate transition rather than silent drift. + +--- + +## Why this is the right call today + +- **The lift hasn't arrived.** ADR-0047 measured 0 Δ on every public- + split metric with the flag flipped. The 6/13 cases that produced a + non-trivial constraint label did not produce a different surface. + Opting `precision_first_v1` into the flag today would not move any + observable cognition-lane number. +- **The infrastructure cost is real.** Pack-to-runtime preference + composition is a load-bearing architectural decision, not a + one-line wiring change. When the lift arrives — when constraint- + aware realizer logic exists downstream — that's the right context + in which to design the composition path, because the requirements + will be clearer. +- **The invariant test is real epistemic work.** Pinning the null- + lift as CI-enforced turns "observed once during ADR-0047" into + "structurally true across the codebase's evolution." If + constraint-aware realizer logic later moves a metric on the flag + flip, the test surfaces it as the transition it is. + +--- + +## What is and isn't open + +### What this ADR does not foreclose + +- A future ADR that wires constraint-aware behaviour into the + realizer or surface-assembly layer. Once that work exists, the + null-lift test in this ADR will fail (intentionally), and a + follow-up ADR will document the transition + design the + pack→runtime composition path. +- Operator opt-in via `RuntimeConfig(forward_graph_constraint=True)` + on a per-call or per-deployment basis remains available; this ADR + only addresses the *default* and the *pack-level* configuration. + +### What this ADR explicitly closes + +- The ADR-0047 follow-up question "should this become default-on or + pack-opt-in?" is answered with **neither, yet**. +- The question "should identity packs carry runtime config + preferences?" is deferred until at least one such preference has + demonstrated lift. + +--- + +## Verification + +``` +tests/test_forward_graph_constraint_wiring.py 5 passed (ADR-0047 wiring) +tests/test_forward_graph_constraint_null_lift.py 2 passed (ADR-0058 invariant) + + test_cognition_lane_metrics_identical_with_flag_flipped + Runs the public cognition split twice (flag OFF, flag ON) and + asserts intent_accuracy / surface_groundedness / term_capture_rate / + versor_closure_rate are pair-wise identical. ~3s wall-time. + + test_default_config_keeps_flag_off + RuntimeConfig().forward_graph_constraint is False — the + production-default contract. +``` + +The non-negotiable field invariant (`versor_condition(F) < 1e-6`) +is unaffected: this ADR adds no runtime behaviour; it only adds a +regression test and a doctrinal decision. + +--- + +## Cross-References + +- [ADR-0046](./ADR-0046-forward-graph-constraint.md) — the constraint + primitive (`build_graph_constraint`). +- [ADR-0047](./ADR-0047-wire-forward-graph-constraint.md) — the + hot-path wiring + opt-in flag + original A/B characterisation. +- [ADR-0048](./ADR-0048-pack-grounded-surface.md) through + [ADR-0053](./ADR-0053-cognition-lane-closure.md) — the realizer / + surface-assembly work that produced the actual cognition-lane lift, + validating that the gap lived downstream of propagation. +- [ADR-0027](./ADR-0027-identity-packs.md) — the identity-pack + contract whose schema this ADR explicitly chooses not to extend. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index c29f96af..d8030404 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -67,6 +67,7 @@ ADRs record significant architectural decisions: what was decided, why, what alt | [ADR-0055](ADR-0055-inter-session-memory-discovery-promotion.md) | Inter-session memory: reviewed discovery promotion (phased design — DiscoveryCandidate, TeachingChainProposal, replay-equivalence gate); Phase A + Phase B Accepted | **Phase A + B Accepted**; C–E Proposed (2026-05-18) | | [ADR-0056](ADR-0056-contemplation-loop-c1.md) | Contemplation loop (Phase C1): question decomposition, polarity (affirms/falsifies/undetermined), claim_domain typing (factual/relational/evaluative), sync-only by design | **Accepted** (2026-05-18, implemented `4eecf73`) | | [ADR-0057](ADR-0057-teaching-chain-proposal-review.md) | Teaching-chain proposal + review + replay-equivalence gate (Phase C2): the only path to active-corpus extension; eligibility predicate; auto-reject on metric regression; operator accept/reject/withdraw; append-only proposal log | **Accepted** (2026-05-18) | +| [ADR-0058](ADR-0058-forward-graph-constraint-status.md) | `forward_graph_constraint` remains opt-in default-`False`; no identity pack flips it on; ADR-0047 null-lift on cognition lane promoted to CI-enforced invariant (regression test); identity-pack→`RuntimeConfig` composition deferred until at least one such preference shows lift | **Accepted** (2026-05-18) | --- diff --git a/tests/test_forward_graph_constraint_null_lift.py b/tests/test_forward_graph_constraint_null_lift.py new file mode 100644 index 00000000..dc8f42ab --- /dev/null +++ b/tests/test_forward_graph_constraint_null_lift.py @@ -0,0 +1,73 @@ +"""ADR-0058 — pin the null-lift property of ``forward_graph_constraint``. + +ADR-0047 characterised the cognition-lane A/B between +``forward_graph_constraint`` OFF and ON and recorded the finding: + + intent_accuracy / surface_groundedness / term_capture_rate / + versor_closure_rate are byte-identical with the flag flipped, + while 6/13 cases produce a non-trivial constraint label. + +That finding is what scoped ADR-0048 through ADR-0053 — the surface- +grounding gap lives downstream of propagation, not in the candidate +set the constraint narrows. ADR-0058 promotes the finding from a +historical observation to a **regression-tested invariant**: if a +future change unexpectedly *does* move a cognition-lane metric on the +flag flip, this test fails and the architectural assumption that the +flag is observably inert on the cognition lane gets re-examined as a +deliberate transition rather than silent drift. + +The test runs the full cognition public split twice — flag OFF vs ON — +and asserts the four watched metrics are pair-wise identical. +""" + +from __future__ import annotations + +from dataclasses import replace + +from core.config import RuntimeConfig +from evals.framework import get_lane, run_lane + + +# The four metrics ADR-0047 reported zero delta on. +_WATCHED: tuple[str, ...] = ( + "intent_accuracy", + "surface_groundedness", + "term_capture_rate", + "versor_closure_rate", +) + + +def _run(config: RuntimeConfig) -> dict[str, float]: + lane = get_lane("cognition") + result = run_lane(lane, version="v1", split="public", config=config) + return {k: float(result.metrics[k]) for k in _WATCHED} + + +def test_cognition_lane_metrics_identical_with_flag_flipped() -> None: + """The ADR-0047 null-lift invariant: every watched metric is + pair-wise identical between flag OFF and flag ON on the public + cognition split. If a future change moves any of these metrics + on the flag flip, the architectural assumption that the constraint + is observably inert on this lane no longer holds — surface that + as a regression rather than silent drift. + """ + off = _run(RuntimeConfig(forward_graph_constraint=False)) + on = _run(replace(RuntimeConfig(), forward_graph_constraint=True)) + + for metric in _WATCHED: + assert off[metric] == on[metric], ( + f"ADR-0058 null-lift invariant broken on metric {metric!r}: " + f"flag OFF={off[metric]} vs flag ON={on[metric]}. Either a " + f"deliberate downstream wiring change closed the surface-" + f"grounding gap (in which case update ADR-0058 to mark the " + f"transition), or a regression slipped in (in which case " + f"find it before merging)." + ) + + +def test_default_config_keeps_flag_off() -> None: + """The default ``RuntimeConfig().forward_graph_constraint`` remains + ``False``. This is the contract production callers rely on; any + flip of the default would change the runtime's default behaviour + on inputs whose intent-derived graph is non-trivial.""" + assert RuntimeConfig().forward_graph_constraint is False