S2 Step 10 — Analyzer Delta Verification (verify-delta) - #290
Merged
Merged
Conversation
…sher
Slice 1 of S2 step 10 (analyzer delta verification), the accepted contract's
red-to-green slice 1. Adds ownlang/fix_delta.py with the pure, SDK-free core:
- the two-representation delta classifier (Section 8): the subscription
finding-id equations, the complete core OWN001 multiset delta
(P_all - B_all == empty, B_all - P_all == R_C), the OWN050 multiset
(P50 - B50 == empty), and the analyzer-to-id bridge that fails closed as
ANALYSIS_IDENTITY on an unbridged / mixed-action / ambiguous candidate;
- the closed core.json schema parser (LA4) with strict field types
(advisory is a required boolean; canonical relative file identity);
- the canonical evidence serializer (sorted keys, trailing newline);
- _publish_delta (LA3): ONE atomic rename of the claimed private workdir to
OUTPUT_DIR, so either OUTPUT_DIR is absent or holds the complete
delta-result.json — nothing runs after the rename.
Reuses the frozen Step 9 helpers by import (never rewrites them). No CLI wiring,
extractor, or core subprocess yet (later slices). tests/test_verify_delta.py
covers the classifier's pass/fail matrix, the core.json schema, and publication.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…ate binding
Slice 2. Adds to ownlang/fix_delta.py:
- load_authority(): reuses the frozen validate_gate_authority to restate the
plan+candidates authority, then enforces the LA2 OWN001-only scope guard —
every accepted candidate must carry diagnostic_code == "OWN001", so a legal
S0 OWN014 (capture) candidate, or a mixed OWN001+OWN014 input, fails closed
as ANALYSIS_SCOPE BEFORE any analyzer runs. OWN014 delta verification is a
future, separate scope.
- bind_gate(): the mandatory Step 9 binding. Validates the supplied
gate-result.json to the exact frozen eleven-key / ten-gate shape (only the
three git gates may be not_applicable, together), reconstructs the expected
evidence from THIS plan+candidates+bundle, and requires BOTH semantic
equality and canonical bytes. Any deviation is GATE_BINDING.
Tests build realistic candidates/plan/gate via the frozen Step 9 producers and
cover the OWN001-only guard and the full gate-binding tampering table (fail
status, unknown key, missing gate, wrong target, illegitimate not_applicable,
split git gates, non-canonical bytes).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…int, baseline authority
Slices 3-5. Adds to ownlang/fix_delta.py the fresh, isolated core-analyzer
subprocess and its bindings:
- RUN_CORE_SOURCE: the deterministic runner. It runs under `python -S -B -E`
with the snapshotted ownlang package on sys.path[0], self-fingerprints after
import (every ownlang module must resolve physically inside the snapshot,
else exit 3), runs the real check_facts + collect_candidates, and emits the
closed core.json (all_own001 / own050 / fix_eligible_subscriptions).
- materialize_core(): snapshots the whole ownlang package (excluding
__pycache__ / *.pyc / *.pyo) into WORK/core/ownlang, writes the runner, and
hashes its bytes as core_runner_sha256, verified immediately (LA1). Returns
the core fingerprint (ownlang_manifest_sha256, ownlang_files, runner sha).
- resolve_python(): snapshots + identifies sys.executable (LA5).
- run_core(): re-verifies the runner bytes before launch (LA1), runs the
subprocess, and parses the schema-checked core.json — exit 3 is
TOOLCHAIN_BINDING (core_analyzer_binding), any other failure is the per-image
BASELINE_ANALYSIS / POSTIMAGE_ANALYSIS.
- check_baseline_authority(): every accepted candidate reproduced exactly over
the baseline for all 18 authoritative fields, else ANALYSIS_SCOPE.
- check_target_identity(): every finding carries file == the target rel.
The fixture test drives the REAL subprocess (no dotnet) end-to-end over
synthetic --fix-candidates facts: the mixed delta (OnA converted, OnB
preserved), baseline-authority mismatch, malformed facts, and a runner mutation
caught before launch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…re, orchestration + CLI
Slices 6-9. Completes the ownlang/fix_delta.py pipeline and wires the CLI:
- snapshot_reference_closure(): ordered one-DLL-per-slot materialization
(caller-dir order then canonical byte order), so first-simple-name-wins is
preserved and filesystem enumeration order never leaks into the analyzer;
ordered reference_closure evidence.
- snapshot_extractor_deployment(): TOCTOU-closed snapshot of the whole
deployment into WORK/toolchain, executes the COPY; ordered manifest.
- resolve_runtime(): proves the EXACT requested runtime is installed under
DOTNET_ROLL_FORWARD=Disable (dotnet --list-runtimes), hashes its manifest,
and returns {framework_name, framework_version, tfm, runtime_manifest_sha256}
plus dotnet_version + dotnet_host_sha256 (LA D4).
- extract_image(): the fixed `dotnet exec <dll> extract ... --fix-candidates
--weak-subscribe <plan target> --ref-dir <slot>...` per image.
- build_evidence(): the full delta-result.json (exact seventeen check names).
- run_verify_delta(): the hermetic orchestration — snapshot inputs, restate
authority + OWN001-only guard, bind the step 9 evidence, snapshot the
extractor + core + python + runtime, analyze preimage/postimage in isolated
workspaces (runner re-verified before each and after the last, LA1), verify
the delta, and publish atomically.
Adds the `own-fix subscriptions verify-delta` CLI verb (--gate mandatory, no
--config, repeated singleton flags rejected) and the single spec/CLI.md row.
Offline tests cover the reference-closure ordering, evidence assembly (seventeen
checks), and bundle layout; the extractor path itself is the Tier-B CI job.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…s -> delta + CI Adds tests/test_verify_delta_tierb.py: the only tier that proves the analyzer-semantic claim with the REAL Roslyn extractor. It extracts a baseline (two INotifyPropertyChanged `+=` leaks) and a postimage (one converted to the accepted weak wrapper), drives the same snapshotted-ownlang fresh core subprocess Step 10 uses, and asserts the OWN001 delta (OnA converted gone, OnB manual preserved). It SKIPS cleanly when dotnet is absent (the Tier-A tests job), so the offline suite stays green; the wpf-extractor CI job runs it for real (new step). The Tier-B test intentionally drives the extractor directly (dotnet exec / run), NOT through run_verify_delta.resolve_runtime — see the PR limitation note: the locked DOTNET_ROLL_FORWARD=Disable + exact-version rule cannot match a runtimeconfig pinned at x.0.0 when only patch runtimes are installed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3fa49b7bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| exe = sys.executable | ||
| if not exe: | ||
| raise DeltaError(TOOLCHAIN_BINDING, "no Python executable to run the core subprocess") | ||
| data = _snapshot(exe, TOOLCHAIN_BINDING, "python executable") |
There was a problem hiding this comment.
When Own.NET runs under a pyenv or virtualenv interpreter, sys.executable is commonly a symlink, but _snapshot rejects symlinks. Consequently resolve_python() raises GateError before any analysis; in the repository's current pyenv environment this makes python tests/test_verify_delta.py fail with a traceback and makes the new CLI unusable. Resolve and validate the physical executable path, then use that same path for fingerprinting and execution.
Useful? React with 👍 / 👎.
|
|
||
| r_c: Counter[str] = Counter() | ||
| consumed: Counter[tuple[str, ...]] = Counter() | ||
| for fid in convert: |
There was a problem hiding this comment.
The core-observation bridge is applied only to convert, so a manual candidate needs a collector record but no corresponding baseline all_own001 finding. This occurs for the repository's real returned_fresh path: the extractor still attaches a fix record (Program.cs:5150-5175) while the core deliberately drops that subscription (ownir.py:964-974). Choosing manual_review for it therefore publishes a passing result claiming the OWN001 was preserved even though the real analyzer never reported one; every accepted ID, including manual IDs, must be bridged before the delta can be certified.
Useful? React with 👍 / 👎.
Comment on lines
+126
to
+129
| except (RuntimeError, OSError) as exc: | ||
| print(f"verify-delta (Tier B): SKIP (extractor unavailable: {exc})") | ||
| return 0 | ||
|
|
There was a problem hiding this comment.
In the wpf-extractor CI job, dotnet is explicitly installed and this test is intended to gate the real extractor/core path, but any RuntimeError from _extract is converted into a successful skip. An extractor build failure, crash, or regression specific to this fixture therefore leaves the CI step green without exercising the advertised Tier-B assertion. The earlier no-dotnet check already handles genuinely unsupported environments, so extraction failures after that point should fail the test.
Useful? React with 👍 / 👎.
…core.json, publish) R1 runtime selection: replace the unusable exact-runtimeconfig rule. Parse the requested minimum (stable major.minor.patch), select the highest installed STABLE patch of the same major.minor that is >= the minimum, publish both requested_framework_version and selected_framework_version, pin it on the extractor argv (`--fx-version <selected> --roll-forward Disable`, off the env), and fingerprint the SELECTED runtime directory. _select_runtime is pure and unit tested (8.0.0 requested + only 8.0.28 -> selects 8.0.28; highest patch; lower / different-minor / prerelease-only / none -> TOOLCHAIN_BINDING). R2 check-state + revalidation: build_evidence now takes the set of executed checks and refuses to publish any it did not run. Before each image and after the postimage, _revalidate_toolchain re-derives every MATERIALIZED hash (extractor deployment, reference slots, ownlang package, core runner, python, dotnet host, selected runtime) and _isolation_verify re-reads the pristine target + .git/index + .git/config; only then are toolchain_binding / core_analyzer_binding / isolation marked pass. Mutation regressions added. R3 image-level bridge: validate_image_bridge groups eligible OWN001 facts and core OWN001 observations by K for BOTH images and requires exact multiset cardinality + a single distinct observation shape; every accepted id (C and M) must map through a validated baseline group; R_C is built only after the complete baseline bridge passes. Regressions for missing/surplus/mixed cases. R4 closed core.json byte protocol: _load_core requires the trailing newline and canonical_bytes(parsed) == the original bytes; the runner emits advisory VERBATIM (no bool() coercion) and _validate_record type-checks all 18 fields (span, teardown, actions, ordinal). Non-canonical / missing-newline / unknown-key / wrong-type / advisory-coercion regressions added. R5 publication cleanup: a pre-rename failure rmtree's the workdir WITHOUT ignore_errors and a cleanup failure is itself PUBLICATION; a rename fault is PUBLICATION; nothing runs after a successful rename. Deterministic cleanup-failure regression added. R6 target identity: a foreign / absolute / escaping / image-mismatched target-file identity is ANALYSIS_SCOPE (not BASELINE/POSTIMAGE_ANALYSIS). R8 (partial): green lint (ruff) and mypy --strict; fix the Linux-CI crash where sys.executable / the dotnet host are symlinks — trusted system binaries are now hashed by following the symlink to their real target (_hash_resolved), distinct from the strict _snapshot for untrusted caller inputs. The runner writes core.json in binary (LF everywhere). Frozen Steps 8/9 remain untouched. Offline suite: verify-delta 90/90; Step 8 79/79; Step 9 69/69; harness 25/25. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
R7: rewrite the Tier-B test to exercise ONLY the public pipeline, never classify_delta / run_core / a private helper as its proof: real extractor -> candidates -> validate-plan -> apply (Owen rewriter) -> gate -> `own-fix subscriptions verify-delta` CLI -> published delta-result.json using the copied extractor deployment, the amended runtime resolver, real Roslyn extraction, and the real fresh snapshotted Python core. Cases: all-convert, manual-only, mixed (each exit 0 + full published-schema validation, including requested/selected runtime versions); deterministic byte-identical evidence across two independent invocations; an OWN014 candidate -> ANALYSIS_SCOPE; and forged-but-gate-valid bundles whose postimage introduces a new leak (-> NEW_OWN001) or an unresolved subscription (-> NEW_OWN050). Gating (R7): REQUIRED when OWN_TIERB_REQUIRED=1 — set in the wpf-extractor CI job, where a missing dotnet / extractor / execution failure is a FAILURE, not a skip. Any other context (the Tier-A tests job, the pack job, a local no-dotnet run) is the explicit non-required mode and skips cleanly, so the offline suite stays green. R8: the wpf-extractor CI step now runs the required Tier-B; combined with the lint / mypy / symlink fixes in the previous commit, the CI matrix is green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
|
@coderabbitai review |
… not INFRASTRUCTURE The reused fix_gate snapshot/publish helpers raise GateError (which carries the same stable .category as DeltaError). The verify-delta CLI only caught DeltaError, so a missing/symlinked input file leaked its GateError to the generic handler and was reported as INFRASTRUCTURE instead of its real category (e.g. INPUT_LAYOUT). The CLI now catches (DeltaError, GateError) and prints exc.category. Regression: a verify-delta invocation with missing inputs exits 2 with a controlled refusal category, never INFRASTRUCTURE. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…wn validation, Tier B asserts
C1: the fresh core subprocess no longer inherits the caller's HOME. _core_env
now takes the work root and sets HOME=WORK/home, XDG_CACHE_HOME=WORK/home/.cache,
and TMPDIR/TEMP/TMP=IMAGE_WORKSPACE (the workspace-local home/.cache are created
before launch); host HOME is not copied. Keeps python -S -B -E. Regression: the
child sees the workspace-local HOME, host HOME is absent, and the redirected
home/cache mean a core run leaves no artifact under the real user home.
C2: _validate_record now fully closes the nested teardown protocol — exact
{status, candidates}; status in {none, exact, ambiguous}; each candidate an exact
{source, handler, match, span} object with string source/handler/match and the
frozen six-int span (no booleans). Malformed nested values refuse the per-image
BASELINE_ANALYSIS / POSTIMAGE_ANALYSIS. Regressions for non-object, missing key,
extra key, non-string, malformed span, and boolean span value.
C3: the full public-CLI Tier B now parses each published delta-result.json and
asserts case-specific semantics for mixed / all-convert / manual-only (convert &
manual counts, removed==convert, preserved==manual, removed_all_own001 count,
postimage subscription ids, baseline==postimage for manual-only, git_gates_status,
and vacuous idempotence), retaining the schema / determinism / OWN014 /
NEW_OWN001 / NEW_OWN050 checks.
Frozen Steps 8/9 untouched. verify-delta 104/104; Tier B 28/28; Step 8 79/79;
Step 9 69/69; harness 25/25; ruff + mypy clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
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.
S2 Step 10 — Analyzer Delta Verification
Implements the accepted S2 Step 10 contract (
S2_STEP10_LOCKED_CONTRACT.txt, shab6eb6ce3…) with locked amendments LA1–LA5, review round R1–R8, and final cleanupC1–C4. Step 10 is the analyzer-semantic gate after the frozen Steps 8/9: it re-runs
Own.NET's real core analyzer — from a snapshotted
ownlangpackage in a fresh,isolated
python -S -B -Esubprocess, the extractor from a snapshotted deployment onthe selected runtime — over the pristine preimage and the accepted postimage, and proves
the OWN001 delta matches the plan (converted gone, manual preserved, no new OWN001 of any
lane, no new OWN050). New CLI:
own-fix subscriptions verify-delta(--gatemandatory, no--config).Runtime selection (R1)
The extractor's
runtimeconfigpins a minimum framework version (e.g.8.0.0), which isalmost never the installed patch.
resolve_runtimeselects the highest installed stablepatch of the same major.minor that is ≥ the requested minimum and pins it on the argv
(
dotnet exec --fx-version <selected> --roll-forward Disable), publishing bothrequested_framework_versionandselected_framework_versionand fingerprinting theselected runtime directory. In CI/local: requested
8.0.0→ selected installed8.0.28.Tests (all green)
tests/test_verify_delta.py(no dotnet): 104/104 — the classifier pass/failmatrix, the closed
core.jsonbyte protocol, the atomic publisher, the OWN001-only guard,the exact Step 9 gate binding, the reference-closure snapshot, evidence assembly, the
real fresh core subprocess end-to-end, and the R1–R6 + C1–C2 regressions.
tests/test_verify_delta_tierb.py(full public CLI: bundle → Step 9 gate →own-fix subscriptions verify-delta→ publisheddelta-result.json): 28/28 — all-convert,manual-only, mixed (each with schema + case-specific semantic assertions), determinism
(byte-identical evidence across two invocations), OWN014 → ANALYSIS_SCOPE, NEW_OWN001,
NEW_OWN050. REQUIRED in the
wpf-extractorCI job (OWN_TIERB_REQUIRED=1); skips in theexplicit non-required mode elsewhere.
harness 25/25; ruff + mypy
--strictclean; GitHub Actions green.Review resolution
R1 runtime selection · R2 executed-check tracking + toolchain/isolation revalidation ·
R3 image-level bridge (both images/actions) · R4 closed core.json byte protocol (advisory
verbatim) · R5 honest publish cleanup · R6 target-identity → ANALYSIS_SCOPE · R7 full
public-CLI Tier B · R8 CI green. Final cleanup C1 core-subprocess HOME/cache redirect ·
C2 deep teardown-candidate validation · C3 case-specific Tier B assertions · C4 this
description.
Step 10 implementation only; Steps 11–12 not started. Do not merge — subject to arbiter
final approval.
🤖 Generated with Claude Code