Skip to content

Commit 514289d

Browse files
PhysShellclaude
andcommitted
fix(extractor): --fix-candidates no longer drops orphaned_awaitables (OWN053)
`--fix-candidates` is additive: it adds fix metadata and changes nothing else. It removed a section. The same input, the same build: --flow-locals orphaned_awaitables: 5 sites --flow-locals --fix-candidates orphaned_awaitables: absent so every OWN053 advisory disappeared from a run that asked for fix candidates (corpus/ownership-lab/h29/fx/Orphan.cs: 5 advisories without the flag, 0 with it, on both engines). Root cause: the extractor built its envelope in three places, picked by a nested conditional — `--fix-candidates` ? A : orphans ? B : C. The `--fix-candidates` envelope was written before `orphaned_awaitables` existed and was never given it. Each additive section had to be remembered in every envelope, and one was not. The fix is the mechanism, not the missing line: ONE envelope, built once, in the key order the facts have always had; a section that is not always there is one conditional line. There is no second envelope for the next section to be forgotten in. Output without the flag is unchanged byte for byte, and so is every flag-on output that had nothing to lose: 256 outputs compared with main's build (every C# input CI scans and the two OWN053 fixtures, with and without --flow-locals, flag on and off) — 254 byte-identical, 2 repaired (they gain exactly the section they used to drop), 0 other. Why the S0 control did not see it. The invariant it holds was already the right one — flag-on minus the S0 fields == flag-off, the whole document. The document was the problem: FixCandidatesSample.cs has subscriptions and no other section, and CI scans it without --flow-locals, so nothing the flag could drop was ever there. Now: * tests/fixtures/fix_candidates/AdditiveSections.cs makes every top-level section the extractor can write non-empty under --flow-locals (both OWN053 families included, with no reference directory); * `check_fix_candidates_facts.py --additive-sections` holds the equality on it, reads the section list off the extractor's own envelope (`facts["<key>"] = ...`) and refuses a pair that leaves a section out — so a section added to the extractor is red until the fixture exercises it — and requires the reference to give the same findings for both documents; * the CI step "S0 fix-candidates — extractor metadata (Part A)" runs it. Negative controls: on main's extractor the new check fails (`orphaned_awaitables` is in the flag-off facts and MISSING from the flag-on facts; 3 findings with the flag, 5 without); with the section dropped under the flag in this tree it fails the same way and passes again once restored; an envelope key the fixture does not exercise fails it by name; and the old sample, given to the new mode, is refused as not exercising `services`, `functions` and `orphaned_awaitables`. No OwnIR vocabulary or version change (OWNIR_VERSION 1, LOWERED_VERSION 2), no change to the OWN053 rule, to the state-protocol lowering or to the measurement instrument. spec/OwnIR.md §2 states the producer-side rule. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 7053439 commit 514289d

5 files changed

Lines changed: 294 additions & 48 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,6 +2205,22 @@ jobs:
22052205
echo "FAIL: flag-off facts drifted from the pre-S0 golden (byte parity broken)"; exit 1
22062206
fi
22072207
echo "OK: fix-candidate metadata correct; flag-off is byte-identical to the pre-S0 golden"
2208+
# ADDITIVE means the flag never REMOVES anything either, and the sample above
2209+
# cannot show that: it has subscriptions and no other section. For as long as
2210+
# `orphaned_awaitables` existed, the flag-on envelope dropped it — every OWN053
2211+
# site gone from the facts — with this step green. So the same invariant
2212+
# (flag-on minus the S0 fields == flag-off, the WHOLE document) is checked on a
2213+
# fixture that makes every top-level section the extractor can write non-empty
2214+
# under --flow-locals. The checker reads that section list off the extractor's
2215+
# own envelope and refuses a pair that leaves one out, so a new section cannot
2216+
# arrive without being covered here.
2217+
ad=tests/fixtures/fix_candidates/AdditiveSections.cs
2218+
dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \
2219+
"$ad" --flow-locals --fix-candidates -o "$RUNNER_TEMP/ad_on.json"
2220+
dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \
2221+
"$ad" --flow-locals -o "$RUNNER_TEMP/ad_off.json"
2222+
python tests/check_fix_candidates_facts.py --additive-sections \
2223+
"$RUNNER_TEMP/ad_on.json" "$RUNNER_TEMP/ad_off.json"
22082224
22092225
# S0 Part B: the `own-fix subscriptions candidates` collector turns the fix
22102226
# metadata into a deterministic candidates.json (analysis-only). Reuses fc_on.json.

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 32 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7280,47 +7280,39 @@ or ImplicitObjectCreationExpressionSyntax } init
72807280
methods_flow_analysed = statMethodsAnalysed,
72817281
methods_skipped_unmodelled = statMethodsSkipped,
72827282
};
7283-
// `fix_candidates_version` is a top-level ADDITIVE metadata field, present ONLY under
7284-
// --fix-candidates; it does not move `ownir_version` (the fact-schema vocabulary is
7285-
// unchanged — no new resource-kind or analysis-routing value). Without the flag the
7286-
// object is byte-for-byte the pre-S0 shape.
7283+
// ONE envelope, built once, in the key order the facts have always been written. A section
7284+
// that is not always there is ONE conditional line here — never a second envelope. There
7285+
// used to be three, picked by a nested conditional (`--fix-candidates` ? A : orphans ? B : C),
7286+
// and each additive section had to be remembered in every one of them: the `--fix-candidates`
7287+
// envelope was written before `orphaned_awaitables` existed and never got it, so the flag
7288+
// that only ADDS fix metadata silently removed every OWN053 site from the facts.
72877289
//
7288-
// Every envelope below stamps the SAME `ownir_version`, the core's current one
7289-
// (ownlang/ownir.py OWNIR_VERSION; tests/test_ownir.py reads every stamp in this file).
7290-
object facts = emitFixCandidates
7291-
? new
7292-
{
7293-
ownir_version = 1,
7294-
fix_candidates_version = 1,
7295-
module = "Extracted",
7296-
components,
7297-
services = factServices,
7298-
functions = flowFunctions,
7299-
stats = factStats,
7300-
}
7301-
// OWN053 (promoted from ownership-semantics-lab H-29): an ADDITIVE top-level list of orphaned awaitables from which
7302-
// both engines mint the advisory; absent when there is no site, so such a document stays byte-identical to the
7303-
// pre-OWN053 shape (and `ownir_version` does not move: the field is additive, like `fix_candidates_version`).
7304-
: OrphanedAwaitables.Sites.Count > 0
7305-
? new
7306-
{
7307-
ownir_version = 1,
7308-
module = "Extracted",
7309-
components,
7310-
services = factServices,
7311-
functions = flowFunctions,
7312-
stats = factStats,
7313-
orphaned_awaitables = OrphanedAwaitables.Sites.OrderBy(o => JsonSerializer.Serialize(o), StringComparer.Ordinal).ToList(),
7314-
}
7315-
: new
7316-
{
7317-
ownir_version = 1,
7318-
module = "Extracted",
7319-
components,
7320-
services = factServices,
7321-
functions = flowFunctions,
7322-
stats = factStats,
7323-
};
7290+
// Every section is written as `facts["<key>"] = ...` and nowhere else: the S0 additivity
7291+
// check (tests/check_fix_candidates_facts.py) reads the section list off these lines, so a
7292+
// section added here is one its fixture must exercise.
7293+
//
7294+
// `ownir_version` is the core's current one (ownlang/ownir.py OWNIR_VERSION). It is spelled
7295+
// as this one literal on purpose: tests/test_ownir.py (IR2) reads every stamp in this file.
7296+
const int ownir_version = 1;
7297+
var facts = new Dictionary<string, object?>();
7298+
facts["ownir_version"] = ownir_version;
7299+
// S0: ADDITIVE metadata, present ONLY under --fix-candidates. It does not move the version
7300+
// (no new resource-kind or analysis-routing value), and removing the S0 fields from a
7301+
// flag-on document gives the flag-off document — every other section included
7302+
// (tests/check_fix_candidates_facts.py holds exactly that).
7303+
if (emitFixCandidates)
7304+
facts["fix_candidates_version"] = 1;
7305+
facts["module"] = "Extracted";
7306+
facts["components"] = components;
7307+
facts["services"] = factServices;
7308+
facts["functions"] = flowFunctions;
7309+
facts["stats"] = factStats;
7310+
// OWN053 (promoted from ownership-semantics-lab H-29): an ADDITIVE top-level list of orphaned
7311+
// awaitables from which both engines mint the advisory. Absent when there is no site, so such
7312+
// a document stays byte-identical to the pre-OWN053 shape.
7313+
if (OrphanedAwaitables.Sites.Count > 0)
7314+
facts["orphaned_awaitables"] = OrphanedAwaitables.Sites
7315+
.OrderBy(o => JsonSerializer.Serialize(o), StringComparer.Ordinal).ToList();
73247316
// P-037 A2.1: a sidecar record that does not satisfy its own vocabulary is a producer
73257317
// defect, and a producer defect must not become a facts file. Refuse the whole run
73267318
// (exit 2, the launcher's "extraction failed, no verdict was produced" tier) rather

‎spec/OwnIR.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ A newer extractor that introduces either against an un-bumped core therefore fai
8989
the run instead of mis-analyzing it — which is why both **must** bump
9090
`OWNIR_VERSION` per the table above.
9191

92+
**Additive is a promise about the producer too.** A producer option that adds
93+
metadata must change nothing else in the document it writes. The extractor's
94+
`--fix-candidates` adds `fix_candidates_version`, the component shape fields and
95+
a `fix` block per subscription; take those out of a flag-on document and what is
96+
left **is** the flag-off document — every section, `orphaned_awaitables` (§9)
97+
included. The extractor builds its envelope once for that reason (one conditional
98+
line per optional section, never a second envelope), and
99+
`tests/check_fix_candidates_facts.py --additive-sections` holds the equality on a
100+
fixture that exercises every top-level section the extractor can write.
101+
92102
## 3. What OwnIR is not
93103

94104
Verdict logic never lives in a frontend. The core's diagnostics (OWN0xx) come

‎tests/check_fix_candidates_facts.py‎

Lines changed: 143 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,47 @@
1-
"""Assert the S0 `--fix-candidates` extractor metadata on FixCandidatesSample.cs.
1+
"""Assert the S0 `--fix-candidates` extractor contract at the fact level.
22
33
Not a ``test_*`` (it needs the C# extractor to produce the facts, so CI runs the
4-
extractor first and passes the JSON path). Encodes the Part-A extractor contract
5-
at the fact level; exits non-zero on any violation.
4+
extractor first and passes the JSON paths). Exits non-zero on any violation.
65
76
Usage:
87
python tests/check_fix_candidates_facts.py <fix_on.json> [<off.json>]
8+
python tests/check_fix_candidates_facts.py --additive-sections <on.json> <off.json>
99
1010
fix_on = FixCandidatesSample.cs scanned WITH --fix-candidates
1111
off = the SAME sample WITHOUT the flag (optional; asserts NO fix metadata leaks)
12+
13+
The first form checks the metadata itself on FixCandidatesSample.cs. The second
14+
checks the other half of the contract — the flag only ADDS — on
15+
tests/fixtures/fix_candidates/AdditiveSections.cs scanned with `--flow-locals`,
16+
flag on and flag off:
17+
18+
flag-on minus the S0 fields == flag-off (the whole document)
19+
20+
and refuses a pair that does not exercise every top-level section the extractor
21+
can write (read off the extractor's own envelope), so that the equality cannot
22+
be satisfied by a document with nothing in it. It was: the first form's sample
23+
has subscriptions and no other section, and a flag-on envelope that dropped
24+
`orphaned_awaitables` — every OWN053 site — passed it for as long as it existed.
1225
"""
1326

1427
from __future__ import annotations
1528

1629
import copy
1730
import json
1831
import os
32+
import re
1933
import sys
2034

2135
sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), ".."))
2236

23-
from ownlang.ownir import OWNIR_VERSION
37+
from ownlang.ownir import OWNIR_VERSION, OwnIRError, check_facts
38+
39+
_ROOT = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")
40+
# The ONLY top-level field S0 adds. Everything else under --fix-candidates lives inside
41+
# `components[]` (see `_strip_additive`); a second top-level S0 field is a contract change
42+
# and belongs here, in the open.
43+
_ADDITIVE_TOP_LEVEL = ("fix_candidates_version",)
44+
_EXTRACTOR = os.path.join(_ROOT, "frontend", "roslyn", "OwnSharp.Extractor", "Program.cs")
2445

2546
_ADDITIVE_COMPONENT_KEYS = (
2647
"qualified_name",
@@ -32,9 +53,15 @@
3253

3354

3455
def _strip_additive(facts: dict) -> dict:
35-
"""The flag-ON facts with every S0-additive field removed."""
56+
"""The flag-ON facts with every S0-additive field removed — and nothing else.
57+
58+
This is the whole list of what `--fix-candidates` is allowed to change. It is an
59+
ALLOWLIST of S0's own fields, so the comparison it feeds is over the complete
60+
document: a section the flag drops, reorders or rewrites is a difference, whether
61+
or not anyone thought to name that section in a test."""
3662
f = copy.deepcopy(facts)
37-
f.pop("fix_candidates_version", None)
63+
for k in _ADDITIVE_TOP_LEVEL:
64+
f.pop(k, None)
3865
for c in f.get("components", []):
3966
for k in _ADDITIVE_COMPONENT_KEYS:
4067
c.pop(k, None)
@@ -62,6 +89,111 @@ def _fixes(facts: dict, name: str) -> list[dict]:
6289
return [s["fix"] for s in (comp.get("subscriptions") or []) if s.get("fix")]
6390

6491

92+
def _additivity_problem(on: dict, off: dict) -> str | None:
93+
"""`flag-on minus the S0 fields == flag-off`, or what differs."""
94+
stripped = _strip_additive(on)
95+
detail = []
96+
for k in sorted(k for k in set(stripped) | set(off) if stripped.get(k) != off.get(k)):
97+
if k not in stripped:
98+
detail.append(f"`{k}` is in the flag-off facts and MISSING from the flag-on facts")
99+
elif k not in off:
100+
detail.append(f"`{k}` appears only in the flag-on facts and is not an S0 field")
101+
else:
102+
detail.append(f"`{k}` differs")
103+
if not detail and list(stripped) != list(off):
104+
detail.append(f"the top-level key ORDER differs: {list(stripped)} vs {list(off)}")
105+
if not detail:
106+
return None
107+
return ("flag-on minus the S0 fields must equal flag-off: " + "; ".join(detail)
108+
+ " — --fix-candidates only adds, it never removes or changes another section")
109+
110+
111+
def _extractor_sections() -> list[str]:
112+
"""The top-level keys the extractor's envelope can write, read from its source.
113+
114+
The envelope is built in one place as `facts["<key>"] = ...`. Reading the keys from
115+
there, instead of listing them here, is what makes a NEW section fail this check
116+
until the fixture exercises it."""
117+
with open(_EXTRACTOR, encoding="utf-8") as fh:
118+
keys = re.findall(r'\bfacts\["([a-z_]+)"\]\s*=', fh.read())
119+
return list(dict.fromkeys(keys))
120+
121+
122+
def _findings(facts: dict) -> list[tuple[object, ...]] | str:
123+
try:
124+
return sorted((f.code, f.file, f.line, f.column or 0, f.severity, f.message)
125+
for f in check_facts(facts))
126+
except OwnIRError as e:
127+
return f"refused: {e}"
128+
129+
130+
def additive_sections(on_path: str, off_path: str) -> int:
131+
"""`--additive-sections`: the flag only adds, over EVERY section the extractor writes."""
132+
on, off = _load(on_path), _load(off_path)
133+
fails: list[str] = []
134+
135+
# 1. the pair really is flag-on / flag-off, at the current version
136+
if on.get("fix_candidates_version") != 1 or "fix_candidates_version" in off:
137+
fails.append("the pair is not (flag-on, flag-off): fix_candidates_version must be 1 "
138+
"in the first document and absent from the second")
139+
components = on.get("components")
140+
if not any(s.get("fix") for c in (components if isinstance(components, list) else [])
141+
for s in c.get("subscriptions") or []):
142+
fails.append("the flag-on facts carry no `fix` block: the pair does not exercise S0")
143+
if on.get("ownir_version") != OWNIR_VERSION or off.get("ownir_version") != OWNIR_VERSION:
144+
fails.append(f"ownir_version must be the core's {OWNIR_VERSION} in both documents: "
145+
f"{on.get('ownir_version')!r} with the flag, "
146+
f"{off.get('ownir_version')!r} without")
147+
148+
# 2. not vacuous: every section the extractor CAN write is there, and not empty
149+
sections = [k for k in _extractor_sections() if k not in _ADDITIVE_TOP_LEVEL]
150+
readable = {"ownir_version", "module", "components", "functions"} <= set(sections)
151+
if not readable:
152+
fails.append(f"could not read the extractor's envelope from {_EXTRACTOR} (found "
153+
f"{sections}); the envelope moved — update `_extractor_sections`")
154+
for k in sections:
155+
v = off.get(k)
156+
if k not in off or (isinstance(v, (list, dict)) and not v):
157+
fails.append(f"the fixture does not exercise `{k}`: the extractor can write it, "
158+
f"and the flag-off facts have it "
159+
f"{'empty' if k in off else 'absent'}. "
160+
f"Extend tests/fixtures/fix_candidates/AdditiveSections.cs so the "
161+
f"additivity check covers it")
162+
unknown = sorted(set(off) - set(sections))
163+
if unknown and readable:
164+
fails.append(f"the flag-off facts carry top-level keys this check did not find in the "
165+
f"extractor's envelope: {unknown} — update `_extractor_sections`")
166+
167+
# 3. the invariant itself, over the whole document
168+
problem = _additivity_problem(on, off)
169+
if problem is not None:
170+
fails.append(problem)
171+
172+
# 4. and its consequence: the flag changes no verdict and no advisory
173+
got_on, got_off = _findings(on), _findings(off)
174+
if got_on != got_off:
175+
said = [g if isinstance(g, str) else f"{len(g)} finding(s)" for g in (got_on, got_off)]
176+
fails.append(f"the reference gives different findings with the flag ({said[0]}) "
177+
f"and without it ({said[1]})")
178+
elif isinstance(got_off, str):
179+
fails.append(f"the reference refuses the fixture's facts: {got_off}")
180+
else:
181+
orphans = off.get("orphaned_awaitables") or []
182+
own053 = [f for f in got_off if f[0] == "OWN053"]
183+
if not isinstance(orphans, list) or len(own053) != len(orphans):
184+
fails.append(f"{len(orphans) if isinstance(orphans, list) else '?'} "
185+
f"orphaned_awaitables entr(ies) but {len(own053)} OWN053 advisor(ies)")
186+
187+
if fails:
188+
for fmsg in fails:
189+
print("FAIL:", fmsg, file=sys.stderr)
190+
return 1
191+
print(f"fix-candidates additivity: flag-on minus the S0 fields equals flag-off over "
192+
f"all {len(sections)} sections the extractor writes ({', '.join(sections)}); "
193+
f"the reference gives the same {len(got_off)} finding(s) with and without the flag")
194+
return 0
195+
196+
65197
def main(on_path: str, off_path: str | None) -> int:
66198
on = _load(on_path)
67199
fails: list[str] = []
@@ -247,7 +379,8 @@ def only_fix(name: str) -> dict | None:
247379
# Additivity, positively: strip every additive field from the flag-ON facts and
248380
# the result must EQUAL the flag-off facts (same records, same order, same old
249381
# values) -- enabling the metadata changed nothing pre-existing.
250-
check(_strip_additive(on) == off, "flag-on minus additive fields must equal flag-off")
382+
problem = _additivity_problem(on, off)
383+
check(problem is None, problem or "")
251384

252385
if fails:
253386
for fmsg in fails:
@@ -258,7 +391,9 @@ def only_fix(name: str) -> dict | None:
258391

259392

260393
if __name__ == "__main__":
261-
if len(sys.argv) not in (2, 3):
394+
if len(sys.argv) == 4 and sys.argv[1] == "--additive-sections":
395+
raise SystemExit(additive_sections(sys.argv[2], sys.argv[3]))
396+
if len(sys.argv) not in (2, 3) or sys.argv[1].startswith("--"):
262397
print(__doc__, file=sys.stderr)
263398
raise SystemExit(2)
264399
raise SystemExit(main(sys.argv[1], sys.argv[2] if len(sys.argv) == 3 else None))

0 commit comments

Comments
 (0)