Skip to content

feat(drift): gate semantics v2 — intra-domain cycles, gate_failed, tolerance precedence (#176, #170)#221

Merged
zaebee merged 9 commits into
mainfrom
feat/issue-176-170-gate-semantics
Jun 12, 2026
Merged

feat(drift): gate semantics v2 — intra-domain cycles, gate_failed, tolerance precedence (#176, #170)#221
zaebee merged 9 commits into
mainfrom
feat/issue-176-170-gate-semantics

Conversation

@zaebee

@zaebee zaebee commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #176, closes #170 (roadmap #179 P1 — "make the gate correct across topologies").

Corrected diagnosis first (spec §1.1): the httpx "wall of criticals" was never call cycles — cycle_ratio measured the blast radius of FILE-level IMPORTS cycles, propagated per-node and denominated per-domain, so any single-file domain touching a cross-domain cycle read 1.00. call_cycle_ratio from the issue is explicitly NOT built (it would gate a thing the metric never measured).

Verification highlights

  • Score math byte-identical to main — verified by running BOTH checkouts on the same db: all 5 domains + quotient match to the digit; only statuses re-shade (relative bands), exit stays 0
  • Both issue repros encoded as tests (httpx-shape, owner-api-shape incl. baseline-makes-it-green); #170B repro relational
  • Gate matrix fully covered incl. unenforced-gate_failed observe-only; baseline operators tested for max/min/exact
  • score()-consumer map validated via cgis_analyze_impact (dogfood) — caught the guardian collector consumer the plan missed
  • 890 tests, mypy strict, interrogate 99.5%, ratchets untouched (patterns.yaml diff = comments only)

SDD

5 tasks × (implementer → review), Task 2 with a ULP-deep opus pass; final holistic review READY FOR PR.

  • guardian + colleague review (both repros are their dogfooding)

🤖 Generated with Claude Code

@zaebee

zaebee commented Jun 12, 2026

Copy link
Copy Markdown
Owner Author

/guardian review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — no defects found in this diff.

Reviewed all changes against engineering standards, ontology compliance, and test coverage. The diff implements intra-domain cycle_ratio, gate_failed semantics, hygiene baselines, and per-domain tolerance precedence correctly. All type annotations, docstrings, and test updates align with the spec. No real defects found.


🤖 mistral-medium-latest · 36,624 prompt + 70 completion = 36,694 tokens · graph 7/13 files (54%)

github-actions Bot added a commit that referenced this pull request Jun 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements Drift Gate Semantics v2, introducing intra-domain cycle ratio calculations using Tarjan's SCC, a new gate_failed status for hygiene breaches, support for per-domain hygiene_baseline debt acknowledgments, and relative classification based on effective tolerances. Feedback on the changes identifies a potential KeyError in tarjan_scc within the _intra_domain_cycle_ratio method due to missing keys for sink nodes in the filtered adjacency map, and provides a robust code suggestion to initialize the adjacency dictionary safely.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/cgis/query/fingerprint.py
@zaebee

zaebee commented Jun 12, 2026

Copy link
Copy Markdown
Owner Author

Code review — gate semantics v2 (#176, #170)

Dense, well-specified change; the separate-hygiene-check design and the per-domain tolerance precedence read cleanly, and the SDD trail + dual repros are strong. I ran the finders plus a cgis dogfood pass (ingest → cgis_analyze_impact on DriftScorer.score) to ground-truth the riskier claims. Net: no merge-blockers — three advisory items below, and one scary candidate I want to record as refuted so nobody re-raises it.

✅ Refuted via dogfood — _intra_domain_cycle_ratio is NOT silently 0.0

A finder flagged that _intra_domain_cycle_ratio filters the SCC adjacency to domain_files (FILE/MODULE node ids), but IMPORTS edges are emitted by the raw extractor with target = base_module (e.g. from cgis.query import driftcgis.query), so the adjacency could collapse to empty → always 0.0.

I checked the resolved graph directly instead of the raw extractor:

IMPORTS edges with internal (cgis.*) target:        137
…whose target IS a FILE/MODULE node id:             137   (100%)

The resolver uplifts internal import targets to file-node ids, so SCC detection operates on real file-to-file edges. The finder was reasoning from _python_imports.py alone and missed the resolver step. No action — but worth an integration test that asserts cycle_ratio > 0 from a pipeline-ingested (not hand-built) cyclic fixture, since the existing unit test hand-rolls edges whose targets already match node ids and so can't catch a resolver regression.

🟠 1. violations list contradicts itself for acknowledged hygiene debt

In score(), hygiene stays in the merged constraints = {**hygiene, **template_constraints} and is scored against its original bound by _weighted_constraint_drift (v1) / _gate_drift (v2), which emits a "cycle_ratio X > max 0.0" violation string. Separately, _hygiene_check evaluates the baseline-relaxed bound and emits an "… acknowledged (baseline …)" note. Both land in all_violations.

So a domain that correctly does not gate_failed (debt within baseline) still renders the raw breach and the acknowledgement for the same key in the same report. The gate verdict and drift_score are correct (and genuinely unchanged from main) — this is a report-consistency bug only, but it's exactly the output an operator reads to decide whether to trust the gate. Suggest suppressing the merged-dict violation for any key present in hygiene_eff, so the acknowledged note is the single source of truth.

🟡 2. _classify flipped >=> at the tolerance boundary — gate verdict changes at score == tolerance

if score > tolerance:        return "critical"
if score > 0.75 * tolerance: return "warning"

Old gate tripped on drift_score >= max_drift; the new gate trips only on status == "critical", i.e. score > tolerance. At exactly score == tolerance (e.g. a domain pinned to 0.50 hitting 0.50) the verdict flips fail → pass. The PR says "score math byte-identical … only statuses re-shade" — true for the score, but the gate boundary shifted by one ULP-class. Probably intended (tolerance = max acceptable), but it deserves an explicit boundary test and a one-line note in the spec so it isn't read as accidental.

🟡 3. _baseline_lines ceils for every operator — round-trip only holds for max

_baseline_lines emits _ceil2(measured) for all hygiene keys, with a docstring claiming "the ceil direction is load-bearing" for the round-trip. That's true for max (effective = max(global, ack) ≥ measured ✓), but inverted for min: _apply_baseline takes min(global, ack), and ceil2(measured) > measured can leave the effective floor above the measured value → the proposal gate_faileds on its own graph. exact is worse: _apply_baseline pins effective = ack, and _hygiene_check requires value == bound, so even an improvement later re-fails — a baseline that tightens rather than relaxes.

Latent today (global hygiene: is only cycle_ratio {max} + unresolved_ratio {max}), and the min/exact baseline tests only exercise the consume side, never the init-ontology emit→re-score round-trip. Cheap hardening: pick floor2 for min, skip exact (or assert no non-max hygiene key reaches _baseline_lines), so a future min/exact hygiene rule can't silently break the ratchet.


Consumer map double-checked via cgis_analyze_impact: analyze_drift (threads default_tolerance=max_drift ✓), collector.collect_drift (default 0.50, documented ✓), and ontology_init._fit_templates/_hygiene_score — both construct DomainConfig(drift_tolerance=1.0) explicitly and read only drift_score, so the new default_tolerance never reaches them. All accounted for.

None block merge. #1 is the one I'd fix before merge (operator-facing). 🚀

🤖 via /code-review + cgis dogfood

zaebee added a commit that referenced this pull request Jun 12, 2026
… rounding; boundary pin (#221 review)

Colleague: acknowledged-debt reports no longer show the raw breach alongside
the note; min baselines floor (ceil inverted the round-trip), exact skipped
(pins improvement); score==tolerance boundary pinned+tested. Sonar: approx
float asserts, dead label params dropped. Plus pipeline-ingested cycle
integration test (resolver-uplift guard).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zaebee

zaebee commented Jun 12, 2026

Copy link
Copy Markdown
Owner Author

Re: the dogfood review — disposition (df29366):

  • Refuted finding — thanks for recording it with the 137/137 resolver-uplift evidence; your suggested hardening is in: test_pipeline_ingested_mutual_imports_produce_nonzero_cycle_ratio runs the REAL pipeline (two .py files with mutual imports → ingest → extract) so a resolver regression can no longer hide behind hand-built fixtures.
  • 🟠 1 (contradictory violations) — accepted and fixed: hygiene keys are now single-source — _hygiene_check's breach/acknowledged strings are the only reporting for hygiene-set keys; the merged score path's raw strings are filtered (template-only violations on colliding key names preserved — the unresolved_ratio case keeps its template breach string). Score byte-identity untouched; two exactly-one-entry tests pin it.
  • 🟡 2 (boundary) — accepted as intended + pinned: score == tolerance PASSES (tolerance = maximum acceptable, inclusive); boundary test + one-line spec note added so it reads as deliberate.
  • 🟡 3 (baseline rounding) — accepted, real latent bug: _baseline_lines is now operator-aware — max → ceil2, min → floor2 (your inversion exactly), exact → skipped with an in-code rationale (an exact acknowledgment pins the value and re-fails on IMPROVEMENT — unacknowledgeable by design). 4 emission tests incl. the min/exact behaviors; _baseline_lines takes hygiene as a param now, so future non-max hygiene rules are testable directly.

Sonar round folded into the same commit (approx float asserts; _drift_status_label dead params dropped — now (status) only). gemini's tarjan-KeyError HIGH refuted inline (adj.get + sinks-can't-cycle + prod precedent). 898 tests green.

🤖 Generated with Claude Code

zaebee and others added 9 commits June 12, 2026 23:06
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…is_analyze_impact)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tive status (#170 task 2)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lerance (#170 task 3)

- _drift_status_label: gate_failed branch added FIRST; second param renamed
  max_drift → tolerance; call sites pass r.tolerance; _render_drift_table
  drops its max_drift parameter
- drift --max-drift help: demoted to default tolerance for domains without
  drift_tolerance
- MCP cgis_drift docstring: same semantic note
- guardian/collector.py: deliberate-fallback comment at scorer.score() sites
- tests/unit/conftest.py: promote module_with_funcs from test_fingerprint
  (Sonar dup lesson); test_fingerprint.py updated to import from conftest
- tests/unit/test_cli.py: test_status_label_gate_failed + CLI run-level pair
  (gate failed exit 1 / acknowledged exit 0)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…es (#176/#170 task 4)

- _baseline_lines(): new helper extracted from _domain_entry to avoid C901
  complexity; checks each hygiene constraint in _PARSED_HEADER["hygiene"]
  against the measured fingerprint value using the same operator semantics as
  DriftScorer._hygiene_check (max → measured > bound)
- _domain_entry: calls _baseline_lines(fp) and emits hygiene_baseline block
  with _ceil2'd values and the "acknowledged at baseline by init-ontology"
  comment; applies to BOTH labeled and hygiene-only entries
- _ceil2 direction is load-bearing: floored value would gate_fail the
  proposal on its own graph (round-trip guarantee, spec §2.2)
- test: test_cyclic_domain_proposal_emits_baseline_and_round_trips
  (depth=2 forces unified domain; verifies baseline present + analyze_drift
  any_critical is False + no gate_failed status)
- #174 round-trip test (test_hygiene_only_cyclic_domain_round_trips) stays
  green untouched

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… gate v2 docs (#176/#170 task 5)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… rounding; boundary pin (#221 review)

Colleague: acknowledged-debt reports no longer show the raw breach alongside
the note; min baselines floor (ceil inverted the round-trip), exact skipped
(pins improvement); score==tolerance boundary pinned+tested. Sonar: approx
float asserts, dead label params dropped. Plus pipeline-ingested cycle
integration test (resolver-uplift guard).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zaebee zaebee force-pushed the feat/issue-176-170-gate-semantics branch from df29366 to 6198f27 Compare June 12, 2026 23:09
@sonarqubecloud

Copy link
Copy Markdown

@zaebee

zaebee commented Jun 12, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements Drift Gate Semantics v2, introducing intra-domain cycle ratio calculations, undilutable gate_failed statuses with acknowledgeable hygiene baselines, and per-domain tolerance precedence. Key changes include updating FingerprintExtractor to compute its own intra-domain import cycles via Tarjan's SCC, separating hygiene and template constraint evaluations in DriftScorer, making classification relative to the domain's effective tolerance, and updating ontology_init to emit operator-aware rounded hygiene_baseline blocks for measured breaches. Additionally, the CLI, MCP server, and analyze_drift service were updated to support these status-driven gate semantics. Since no review comments were provided, there is no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@zaebee zaebee merged commit 303a502 into main Jun 12, 2026
3 checks passed
@zaebee zaebee deleted the feat/issue-176-170-gate-semantics branch June 12, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant