P-037 A2.1: guarded-fact sidecar (corrected), richer facts with zero MOS/verdict movement - #363
Merged
Merged
Conversation
…validated and inert
Treatment-only step of the A2 staging (docs/notes/p037-formal-kernel.md §10.3):
the extractor now writes `functions[].guarded_facts`, the raw record of what
flows into each relevant call and what each eligible `if` tests, keyed by
declared parameter ordinal. Neither engine reads it; the legacy `body` stays
authoritative; no path under the A2.0 instrument freeze is touched, so the
after-takes on M1 compare directly against the R baselines with
population_commit = T.
Producer (frontend/roslyn/OwnSharp.Extractor/Program.cs):
- calls[]: one per call a disposable candidate local or an owned parameter
flows into (argument or reduced-extension receiver); site {line, column} of
the invocation plus statement_line as the join identity with the legacy ops;
form statement|initializer|expression; callee/sig or null when unresolved;
first_party; args bound by declared ordinal (named args resolved, extension
receiver = 0, params/ref/out = opaque, unresolved callee = source position).
- arg kinds (raw facts only, §10.2): var{name}, param{source_param[,negated]},
bool_const{value}, null_literal, object_creation, call_result{callee,sig},
opaque. No const-pos/id/neg and no fresh_owned are ever written.
- guards[]: one per eligible `if` on a stable by-value parameter — truth on a
bool, is_null/not_null on a reference or Nullable<T>; G-V4 stability is
whole-body write exposure (assignment, ++/--, ref/out, ref alias, address).
Ineligible guards get no entry: absence is the fail-closed signal.
- Self-check: a sidecar that violates its own vocabulary makes the run refuse
to write facts at all (exit 2), so a producer defect never reaches a door.
Contract (spec/ownir.schema.json, spec/OwnIR.md §5.2, §4.2):
- closed $defs guardedFacts/guardedCall/guardedArg/guardedGuard/sourceSite;
the sidecar is optional and additive, OWNIR_VERSION stays 0.
- sourceSite is deliberately UNBOUND (type only): the doors do not read the
sidecar in A2.1, and §4.2 forbids a schema narrower than the door on a
coordinate the door does not check. tests/test_ownir_defensive_limits.py
classifies it as the one UNBOUND entry; the instrument step that registers
the sidecar at the doors moves it to BOUND.
Controls:
- corpus/p037-shapes: all 11 shapes re-recorded with the sidecar (body ops,
params and both engines' verdicts byte-identical to before), status
pending_a2 -> anchored, and a2_expect contract checks per shape that fail by
name (scripts/p037_fact_shapes.py check).
- tests/test_p037_sidecar.py pins the schema enums against the producer's
vocabularies and the raw-fact boundary.
- scripts/p037_sidecar_inertness.py (CI dogfood step): for every census shape
and the samples tree, the emitted, stripped and contradictory sidecars yield
identical lowered/summaries/verdicts layers on both engines, python == rust,
and the control is non-vacuous (39 sidecars over 12 documents locally).
Known A2.1 boundary, owed by A2.2 (honest emission for every relevant call):
object-creation arguments (`new X(handle, leaveOpen: true)`) are not yet a
call fact; A2.1 covers invocation expressions.
Refs #304
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QfeUtLRm6KFRWcTGQZxQQb
BuildGuardedFacts coupled two independent questions: whether a call is RELEVANT (a tracked/owned handle syntactically flows into it) and how precisely an argument slot can be REPRESENTED (param/var/opaque). Both the params/ref-or-out branch and the unstable-owned-parameter branch in ArgFact hardcoded the handle bit to `false` whenever the slot had to be opaque, so a handle passed through a `params` array, by `ref`/`out`, or as an unstable owned parameter never set `relevant`, and the entire call record silently vanished from guarded_facts instead of surviving with an opaque slot for that argument — contradicting spec/OwnIR.md §5.2, which says an unrepresentable argument becomes opaque, not absent. Fix: evaluate the argument's handle-ness once, independent of how the slot ends up encoded, and thread that value through both branches. Adds three regression shapes to corpus/p037-shapes (a params sink, a ref sink, and an unstable owned parameter flowing to a normal argument), each asserting the call record exists with an opaque slot, not merely that the slot's kind is opaque. Verified against the pre-fix code first (all three reproduced the bug: guarded_facts entirely absent for the method), then against the fix (all three now carry the call record). Full existing 11-shape census, the dedicated sidecar vocabulary test, and the sidecar inertness control (both engines) all still pass unchanged: the sidecar remains semantically inert. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011wKL9pEApQTK4dE3eXv8g8
Supersedes 2cbd6c6 (PR #362), whose source_commit (dce26ed) had the call-relevance bug fixed by the parent commit. Four after-takes (MOS repo, MOS corpus, verdict/python, verdict/rust) at A2.1 treatment commit 5a0de07, population held at terminal-green T (4a8e658), in the same preserved M1 measurement environment used for the T baseline. All four compare UNCHANGED against the R-recorded baseline: MOS repo facts_moved=1, MOS corpus facts_moved=36 (both counts unchanged from the pre-fix run — the fix's edge cases do not appear in this population), both verdict engines report no verdict moved. Evidence-only: no instrument, treatment, spec, or CI file touched; built directly on A', not rebased. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011wKL9pEApQTK4dE3eXv8g8
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: PhysShell/Own.NET/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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 |
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
2 participants
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
dce26ed: the A2.1 treatment — the Roslyn extractor emits a validatedguarded-fact sidecar; the legacy body is unchanged and both consumption
doors ignore it (demonstrably inert).
5a0de07: fix, found by independent review of the original evidence(was PR P-037 A2.1: guarded-fact sidecar, richer facts with zero MOS/verdict movement #362, closed/superseded).
BuildGuardedFactscoupled callRELEVANCE to argument REPRESENTABILITY: a handle passed via
params,ref/out, or as an unstable owned parameter had its relevance bithardcoded to
falsewhenever the slot had to be encodedopaque, sothe entire call record silently vanished from
guarded_factsinsteadof surviving with an opaque slot — contradicting spec/OwnIR.md §5.2
("an unstable parameter argument becomes opaque, not absent"). Adds
three regression shapes (
corpus/p037-shapes/sidecar-relevance-*),each verified against the pre-fix code (reproduced the bug: the call
record was entirely absent) and the fix (call record now present with
an opaque slot).
cd7e020: evidence-only commit recording the M1 after-run comparisonagainst the T-baseline (R, already in
mainvia P-037 A2.0: evidence harness hardening + terminal-green baseline (T, R) #361), taken at thecorrected commit
5a0de07.Test plan (all done pre-merge on this VPS)
samples:
emitted == stripped == contradictoryruff check ./mypy --strict: cleandotnet build(Release): succeeds, only a pre-existing unrelatedwarning
UNCHANGED,facts_moved=1, zero MOS movement on both enginesUNCHANGED,facts_moved=36, zero MOS movement on both enginesUNCHANGED, no verdict moved (137 files)UNCHANGED, no verdict moved (137 files);own-cliqualified sha == executed sha (unchanged from T — fix is C#-only)
facts_movedcounts (1, 36) are unchanged from the pre-fix run: neitherpopulation happens to contain the params/ref/unstable-owned-param edge
cases the fix addresses, so the correctness fix is visible on the three
new regression shapes, not on this measured population.
Known pre-existing observation (not fixed here, tracked in the evidence
manifest):
scope_cache_sites[].filecarries an absolute path ratherthan repo-relative — present since T, out of scope for this PR.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_011wKL9pEApQTK4dE3eXv8g8
🤖 Generated with Claude Code