fix(extractor): --fix-candidates no longer drops orphaned_awaitables (OWN053)

.github/workflows/ci.yml

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

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -5735,7 +5735,7 @@
     .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
     .Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
     .ToList();
 var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);
 var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
 // P-004 WPF profile: widen the reference set with assemblies named by the
 // OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
@@ -7280,47 +7280,39 @@
     methods_flow_analysed = statMethodsAnalysed,
     methods_skipped_unmodelled = statMethodsSkipped,
 };
-// `fix_candidates_version` is a top-level ADDITIVE metadata field, present ONLY under
-// --fix-candidates; it does not move `ownir_version` (the fact-schema vocabulary is
-// unchanged — no new resource-kind or analysis-routing value). Without the flag the
-// object is byte-for-byte the pre-S0 shape.
+// ONE envelope, built once, in the key order the facts have always been written. A section
+// that is not always there is ONE conditional line here — never a second envelope. There
+// used to be three, picked by a nested conditional (`--fix-candidates` ? A : orphans ? B : C),
+// and each additive section had to be remembered in every one of them: the `--fix-candidates`
+// envelope was written before `orphaned_awaitables` existed and never got it, so the flag
+// that only ADDS fix metadata silently removed every OWN053 site from the facts.
 //
-// Every envelope below stamps the SAME `ownir_version`, the core's current one
-// (ownlang/ownir.py OWNIR_VERSION; tests/test_ownir.py reads every stamp in this file).
-object facts = emitFixCandidates
-    ? new
-    {
-        ownir_version = 1,
-        fix_candidates_version = 1,
-        module = "Extracted",
-        components,
-        services = factServices,
-        functions = flowFunctions,
-        stats = factStats,
-    }
-    // OWN053 (promoted from ownership-semantics-lab H-29): an ADDITIVE top-level list of orphaned awaitables from which
-    // both engines mint the advisory; absent when there is no site, so such a document stays byte-identical to the
-    // pre-OWN053 shape (and `ownir_version` does not move: the field is additive, like `fix_candidates_version`).
-    : OrphanedAwaitables.Sites.Count > 0
-    ? new
-    {
-        ownir_version = 1,
-        module = "Extracted",
-        components,
-        services = factServices,
-        functions = flowFunctions,
-        stats = factStats,
-        orphaned_awaitables = OrphanedAwaitables.Sites.OrderBy(o => JsonSerializer.Serialize(o), StringComparer.Ordinal).ToList(),
-    }
-    : new
-    {
-        ownir_version = 1,
-        module = "Extracted",
-        components,
-        services = factServices,
-        functions = flowFunctions,
-        stats = factStats,
-    };
+// Every section is written as `facts["<key>"] = ...` and nowhere else: the S0 additivity
+// check (tests/check_fix_candidates_facts.py) reads the section list off these lines, so a
+// section added here is one its fixture must exercise.
+//
+// `ownir_version` is the core's current one (ownlang/ownir.py OWNIR_VERSION). It is spelled
+// as this one literal on purpose: tests/test_ownir.py (IR2) reads every stamp in this file.
+const int ownir_version = 1;
+var facts = new Dictionary<string, object?>();
+facts["ownir_version"] = ownir_version;
+// S0: ADDITIVE metadata, present ONLY under --fix-candidates. It does not move the version
+// (no new resource-kind or analysis-routing value), and removing the S0 fields from a
+// flag-on document gives the flag-off document — every other section included
+// (tests/check_fix_candidates_facts.py holds exactly that).
+if (emitFixCandidates)
+    facts["fix_candidates_version"] = 1;
+facts["module"] = "Extracted";
+facts["components"] = components;
+facts["services"] = factServices;
+facts["functions"] = flowFunctions;
+facts["stats"] = factStats;
+// OWN053 (promoted from ownership-semantics-lab H-29): an ADDITIVE top-level list of orphaned
+// awaitables from which both engines mint the advisory. Absent when there is no site, so such
+// a document stays byte-identical to the pre-OWN053 shape.
+if (OrphanedAwaitables.Sites.Count > 0)
+    facts["orphaned_awaitables"] = OrphanedAwaitables.Sites
+        .OrderBy(o => JsonSerializer.Serialize(o), StringComparer.Ordinal).ToList();
 // P-037 A2.1: a sidecar record that does not satisfy its own vocabulary is a producer
 // defect, and a producer defect must not become a facts file. Refuse the whole run
 // (exit 2, the launcher's "extraction failed, no verdict was produced" tier) rather

spec/OwnIR.md

@@ -89,6 +89,16 @@ A newer extractor that introduces either against an un-bumped core therefore fai
 the run instead of mis-analyzing it — which is why both **must** bump
 `OWNIR_VERSION` per the table above.
 
+**Additive is a promise about the producer too.** A producer option that adds
+metadata must change nothing else in the document it writes. The extractor's
+`--fix-candidates` adds `fix_candidates_version`, the component shape fields and
+a `fix` block per subscription; take those out of a flag-on document and what is
+left **is** the flag-off document — every section, `orphaned_awaitables` (§9)
+included. The extractor builds its envelope once for that reason (one conditional
+line per optional section, never a second envelope), and
+`tests/check_fix_candidates_facts.py --additive-sections` holds the equality on a
+fixture that exercises every top-level section the extractor can write.
+
 ## 3. What OwnIR is not
 
 Verdict logic never lives in a frontend. The core's diagnostics (OWN0xx) come

tests/check_fix_candidates_facts.py

@@ -1,26 +1,47 @@
-"""Assert the S0 `--fix-candidates` extractor metadata on FixCandidatesSample.cs.
+"""Assert the S0 `--fix-candidates` extractor contract at the fact level.
 
 Not a ``test_*`` (it needs the C# extractor to produce the facts, so CI runs the
-extractor first and passes the JSON path). Encodes the Part-A extractor contract
-at the fact level; exits non-zero on any violation.
+extractor first and passes the JSON paths). Exits non-zero on any violation.
 
 Usage:
     python tests/check_fix_candidates_facts.py <fix_on.json> [<off.json>]
+    python tests/check_fix_candidates_facts.py --additive-sections <on.json> <off.json>
 
   fix_on = FixCandidatesSample.cs scanned WITH --fix-candidates
   off    = the SAME sample WITHOUT the flag (optional; asserts NO fix metadata leaks)
+
+The first form checks the metadata itself on FixCandidatesSample.cs. The second
+checks the other half of the contract — the flag only ADDS — on
+tests/fixtures/fix_candidates/AdditiveSections.cs scanned with `--flow-locals`,
+flag on and flag off:
+
+    flag-on  minus the S0 fields  ==  flag-off          (the whole document)
+
+and refuses a pair that does not exercise every top-level section the extractor
+can write (read off the extractor's own envelope), so that the equality cannot
+be satisfied by a document with nothing in it. It was: the first form's sample
+has subscriptions and no other section, and a flag-on envelope that dropped
+`orphaned_awaitables` — every OWN053 site — passed it for as long as it existed.
 """
 
 from __future__ import annotations
 
 import copy
 import json
 import os
+import re
 import sys
 
 sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), ".."))
 
-from ownlang.ownir import OWNIR_VERSION
+from ownlang.ownir import OWNIR_VERSION, OwnIRError, check_facts
+
+_ROOT = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")
+# The ONLY top-level field S0 adds. Everything else under --fix-candidates lives inside
+# `components[]` (see `_strip_additive`); a second top-level S0 field is a contract change
+# and belongs here, in the open.
+_ADDITIVE_TOP_LEVEL = ("fix_candidates_version",)
+_EXTRACTOR = os.path.join(_ROOT, "frontend", "roslyn", "OwnSharp.Extractor", "Program.cs")
 
 _ADDITIVE_COMPONENT_KEYS = (
     "qualified_name",
@@ -32,9 +53,15 @@
 
 
 def _strip_additive(facts: dict) -> dict:
-    """The flag-ON facts with every S0-additive field removed."""
+    """The flag-ON facts with every S0-additive field removed — and nothing else.
+
+    This is the whole list of what `--fix-candidates` is allowed to change. It is an
+    ALLOWLIST of S0's own fields, so the comparison it feeds is over the complete
+    document: a section the flag drops, reorders or rewrites is a difference, whether
+    or not anyone thought to name that section in a test."""
     f = copy.deepcopy(facts)
-    f.pop("fix_candidates_version", None)
+    for k in _ADDITIVE_TOP_LEVEL:
+        f.pop(k, None)
     for c in f.get("components", []):
         for k in _ADDITIVE_COMPONENT_KEYS:
             c.pop(k, None)
@@ -62,6 +89,111 @@ def _fixes(facts: dict, name: str) -> list[dict]:
     return [s["fix"] for s in (comp.get("subscriptions") or []) if s.get("fix")]
 
 
+def _additivity_problem(on: dict, off: dict) -> str | None:
+    """`flag-on minus the S0 fields == flag-off`, or what differs."""
+    stripped = _strip_additive(on)
+    detail = []
+    for k in sorted(k for k in set(stripped) | set(off) if stripped.get(k) != off.get(k)):
+        if k not in stripped:
+            detail.append(f"`{k}` is in the flag-off facts and MISSING from the flag-on facts")
+        elif k not in off:
+            detail.append(f"`{k}` appears only in the flag-on facts and is not an S0 field")
+        else:
+            detail.append(f"`{k}` differs")
+    if not detail and list(stripped) != list(off):
+        detail.append(f"the top-level key ORDER differs: {list(stripped)} vs {list(off)}")
+    if not detail:
+        return None
+    return ("flag-on minus the S0 fields must equal flag-off: " + "; ".join(detail)
+            + " — --fix-candidates only adds, it never removes or changes another section")
+
+
+def _extractor_sections() -> list[str]:
+    """The top-level keys the extractor's envelope can write, read from its source.
+
+    The envelope is built in one place as `facts["<key>"] = ...`. Reading the keys from
+    there, instead of listing them here, is what makes a NEW section fail this check
+    until the fixture exercises it."""
+    with open(_EXTRACTOR, encoding="utf-8") as fh:
+        keys = re.findall(r'\bfacts\["([a-z_]+)"\]\s*=', fh.read())
+    return list(dict.fromkeys(keys))
+
+
+def _findings(facts: dict) -> list[tuple[object, ...]] | str:
+    try:
+        return sorted((f.code, f.file, f.line, f.column or 0, f.severity, f.message)
+                      for f in check_facts(facts))
+    except OwnIRError as e:
+        return f"refused: {e}"
+
+
+def additive_sections(on_path: str, off_path: str) -> int:
+    """`--additive-sections`: the flag only adds, over EVERY section the extractor writes."""
+    on, off = _load(on_path), _load(off_path)
+    fails: list[str] = []
+
+    # 1. the pair really is flag-on / flag-off, at the current version
+    if on.get("fix_candidates_version") != 1 or "fix_candidates_version" in off:
+        fails.append("the pair is not (flag-on, flag-off): fix_candidates_version must be 1 "
+                     "in the first document and absent from the second")
+    components = on.get("components")
+    if not any(s.get("fix") for c in (components if isinstance(components, list) else [])
+               for s in c.get("subscriptions") or []):
+        fails.append("the flag-on facts carry no `fix` block: the pair does not exercise S0")
+    if on.get("ownir_version") != OWNIR_VERSION or off.get("ownir_version") != OWNIR_VERSION:
+        fails.append(f"ownir_version must be the core's {OWNIR_VERSION} in both documents: "
+                     f"{on.get('ownir_version')!r} with the flag, "
+                     f"{off.get('ownir_version')!r} without")
+
+    # 2. not vacuous: every section the extractor CAN write is there, and not empty
+    sections = [k for k in _extractor_sections() if k not in _ADDITIVE_TOP_LEVEL]
+    readable = {"ownir_version", "module", "components", "functions"} <= set(sections)
+    if not readable:
+        fails.append(f"could not read the extractor's envelope from {_EXTRACTOR} (found "
+                     f"{sections}); the envelope moved — update `_extractor_sections`")
+    for k in sections:
+        v = off.get(k)
+        if k not in off or (isinstance(v, (list, dict)) and not v):
+            fails.append(f"the fixture does not exercise `{k}`: the extractor can write it, "
+                         f"and the flag-off facts have it "
+                         f"{'empty' if k in off else 'absent'}. "
+                         f"Extend tests/fixtures/fix_candidates/AdditiveSections.cs so the "
+                         f"additivity check covers it")
+    unknown = sorted(set(off) - set(sections))
+    if unknown and readable:
+        fails.append(f"the flag-off facts carry top-level keys this check did not find in the "
+                     f"extractor's envelope: {unknown} — update `_extractor_sections`")
+
+    # 3. the invariant itself, over the whole document
+    problem = _additivity_problem(on, off)
+    if problem is not None:
+        fails.append(problem)
+
+    # 4. and its consequence: the flag changes no verdict and no advisory
+    got_on, got_off = _findings(on), _findings(off)
+    if got_on != got_off:
+        said = [g if isinstance(g, str) else f"{len(g)} finding(s)" for g in (got_on, got_off)]
+        fails.append(f"the reference gives different findings with the flag ({said[0]}) "
+                     f"and without it ({said[1]})")
+    elif isinstance(got_off, str):
+        fails.append(f"the reference refuses the fixture's facts: {got_off}")
+    else:
+        orphans = off.get("orphaned_awaitables") or []
+        own053 = [f for f in got_off if f[0] == "OWN053"]
+        if not isinstance(orphans, list) or len(own053) != len(orphans):
+            fails.append(f"{len(orphans) if isinstance(orphans, list) else '?'} "
+                         f"orphaned_awaitables entr(ies) but {len(own053)} OWN053 advisor(ies)")
+
+    if fails:
+        for fmsg in fails:
+            print("FAIL:", fmsg, file=sys.stderr)
+        return 1
+    print(f"fix-candidates additivity: flag-on minus the S0 fields equals flag-off over "
+          f"all {len(sections)} sections the extractor writes ({', '.join(sections)}); "
+          f"the reference gives the same {len(got_off)} finding(s) with and without the flag")
+    return 0
+
+
 def main(on_path: str, off_path: str | None) -> int:
     on = _load(on_path)
     fails: list[str] = []
@@ -247,7 +379,8 @@ def only_fix(name: str) -> dict | None:
         # Additivity, positively: strip every additive field from the flag-ON facts and
         # the result must EQUAL the flag-off facts (same records, same order, same old
         # values) -- enabling the metadata changed nothing pre-existing.
-        check(_strip_additive(on) == off, "flag-on minus additive fields must equal flag-off")
+        problem = _additivity_problem(on, off)
+        check(problem is None, problem or "")
 
     if fails:
         for fmsg in fails:
@@ -258,7 +391,9 @@ def only_fix(name: str) -> dict | None:
 
 
 if __name__ == "__main__":
-    if len(sys.argv) not in (2, 3):
+    if len(sys.argv) == 4 and sys.argv[1] == "--additive-sections":
+        raise SystemExit(additive_sections(sys.argv[2], sys.argv[3]))
+    if len(sys.argv) not in (2, 3) or sys.argv[1].startswith("--"):
         print(__doc__, file=sys.stderr)
         raise SystemExit(2)
     raise SystemExit(main(sys.argv[1], sys.argv[2] if len(sys.argv) == 3 else None))