From b2dd38ae7b653247240a121b5c5e81a7919495f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 07:16:33 +0000 Subject: [PATCH 1/3] =?UTF-8?q?audit:=20add=20PropertyChanged-storm=20prof?= =?UTF-8?q?iler=20(Plan.md=20=C2=A72=20cat.=206)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last runtime detector in the set. Frequency — not correctness — is a runtime property: the static INPC0xx tier (cat. 5) catches a missing nameof or a broken arg, but cannot see that a property fires PropertyChanged thousands of times for one user operation, half of them with no value change, thrashing every binding. This profiler measures that storm. - taxonomy: map RUNTIME-PROPCHANGED-STORM -> category 6 (P2). A storm in the same file as a static INPC0xx hit clusters with it -> high confidence (§3.5). - ingest.py: add propertychanged_storm_to_sarif + --propertychanged-storm (in the mutually-exclusive source group). When the instrumentation resolved a source file the finding keeps it (so it clusters with INPC0xx); otherwise it gets a unique inpc:/// synthetic uri so distinct storming properties stay distinct clusters. Generalized the duplicate-detector's _dup_uri into a shared _synthetic_uri(scheme, ...) helper (heap:// vs inpc://). Selftest proves: below-threshold dropped, level warning, located storm keeps its line + clusters with a static INPC003 (1 high-confidence cluster), distinct properties stay distinct, category 6. - run_static.py: fold propertychanged-storm.sarif into the runtime pickup loop; selftest asserts it. - PropertyChangedStorm/ (C#, Windows/build-required, NOT CI-gated): reads an ETW .etl (Microsoft.Diagnostics.Tracing.TraceEvent) emitted by a diagnostic build's INPC EventSource, aggregates per (type, property), thresholds raises-per- operation, emits JSON. Hardened CLI/exit-code style matching the other harnesses. Selftests: ingest 24/24, run_static 17/17, normalize/score/report unchanged; ruff clean; C# brace/paren-balanced with all TraceEvent tokens present. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE --- audit/runtime/PropertyChangedStorm/Program.cs | Bin 0 -> 11085 bytes .../PropertyChangedStorm.csproj | 23 +++ audit/runtime/README.md | 57 ++++++-- audit/runtime/ingest.py | 131 ++++++++++++++++-- audit/static/run_static.py | 12 +- audit/static/taxonomy/categories.yml | 5 + 6 files changed, 201 insertions(+), 27 deletions(-) create mode 100644 audit/runtime/PropertyChangedStorm/Program.cs create mode 100644 audit/runtime/PropertyChangedStorm/PropertyChangedStorm.csproj diff --git a/audit/runtime/PropertyChangedStorm/Program.cs b/audit/runtime/PropertyChangedStorm/Program.cs new file mode 100644 index 0000000000000000000000000000000000000000..0de2cf83746dcb98f28bd774eeeb0b4cc6915b0f GIT binary patch literal 11085 zcmc&)T~gdg65eM{k?xq7PzT)t+dK0B1_#FAWp)e%FzW|`*eImxMq{knhcMl272)RkFTnP2`^FY+jv()ltsdbYi`Sb6gI(>T_FiP9wZ zKj=hfQP5cNAx^JV9R03%QRB0tlg5jmqvSXD>SGjSX`W7u|2|UFB+X3}i z?Bpq$MY&iS=YsvrlBNrlg(Gm$6vPT22K4FlkglU7ghi~jpo#tJyd~Eac5I(?0^#U7 zOF!#`R5l$Fl)`*-gF^>&bbPwMd4EqPVzrB^e;96K1usj}xa2$2Dqm#U5wp2g*r-Is z%iqBwPzfwTxz@fTb${u|P^MJ~qa<{2@reuorpfcQiH9JZ8}JX=2(gCDhq3zQi1I)u zDvMGIGnh@bG&lypR1{TNaX+ zpQnqA#SHaCEn-kZkSGL;fkplJT+g7&`!TC>Fe1Ch;EW*?4u*6WnVXX7TrJ~Nh4erd z%efwvVvOi-jE%w_Ms#U+J%lPoWdDc5>MMHm=>pbso>DSa(`lxs%D_N;Jg?4v$XJ5$ zZqgX)Pm=c;8#$z zN$Gs=?C6lZY!O#*FZ^(PdUkTU|Chbv4+rnZ=NBhuA8EYHKCY)JyyV9rP0~#4yhx%+ z1VChZo@NLEd%!Hi?v8z{Ja}%+!*N5Xq9ixjVg|#BQ?ux4dop@Ftqx@EIqteZqLCLey*jhhYiFlEWuBYkHT{hQ;KVqgDifZ&9Lk z(+SpzqpcRe`E(2qnybt>9K-aB`;zhg5wW2q!m*gv1n}`FoZ<-wWHVtEz+m!HFnY65 z3&T+YZNP1vA|%i-Ag7s{$!0cya}b|r(JgOT=nSOvQALRdRb%(dD~>GrP>6VWMaWOL zii0eGu|wx~R6Vy7z0wI`I@B`Fg`6Y?y<4ke2?t{C&Db>{pg(})}iy{dD0tnoD2 zvi97|1=&kBA3ntma7Ci<4(dA=^_bBMM51mD{vB-@JCzpz*wlZXlY>O zbi4!VjSCgWOAf>cC;+r*w+O8^^4S<^Bm_J4Bk*LYO==>7bsXl%KMZ0cY=KEg|2~pZ z3`NZw>jWcYW5h(A?RpIh*(%Os4n?rCERX^#r?)ZwYqe?l`R|uV^-Fw{fHTb<))SkgV-~uG~n1K()NuE1K6dVaA%2!B9;`saN3}GrlHRzvYs7zFR zgz7QV`zqIUAgqe<#~<{fV?T*~b~BeepS4f-M%`y+F>JKhE&6$G%o z+@4%iG@=2g*)ES{QYK??b&xqw_K~tp3KbpY#|VKZ*}-gXmL5ZPNYG&Cr@ULk1Kk%TyR9II6la6i zFF$A_wUGxNz~RD(VuPMc>hd?h^8tr<^B*B7MH>|Bo(7& zS)jz>ibDDnRMnxF-sYvRI3u|JRqMI1nnYR7Wq(nG6#+0#kwMX$?kdGz+skcm=7jZ>!@8hW_gH2*r0>7) z_SnF$cz=UYD`Mb~cY4u{Dn6szimMK+-ph{?;~-}F@FxJ+W+Q(&1+@X);UT${&?{wyKyTdMbndMz@vH1{|wNW+Tv2 z$3bYbwI}cJmIm9TLIk;l>LV7q)(i#-f-rQmSR8b++oHjsEi8*RvGbV%mi>rqvF+B( z<$xRL0q2-OZ}&VHwYc$_F9)1#g*NcHO6gn1xsKC%u@sIgkA2G}UF5}Ln@W0@)^X-& z>Tmmq#RrNIAe0gz&bTA+nuM z(jGpDw>)-$q$75VZ+8}!E_ZMWY{Wknvm3sJH(U-}wij)!vTAz~bF1h$A4!E(WJyj? zZJKtNC^+Q)UKo1CLnD@TWTIlo)!W|{>7?dv?UTdtP`ioP-JN}hEiMOc%qesN*z;oW z*+w9Ckhx~H_ClV!RtD(|HC43|9UGpzR+a#>&JA#Ot#X(N5oeZ7`^ah z(m%(Y3Ez+C6DbTlYw!^rqTf*lNoeXHC7AA^1RY|VnxU%%jrx>pEn9)$Rt8PKH{k-o zyN2P16GUwpSPrflqPhuCHT$XUz|ij<20d7wI($SO32`}DxybF|rebJgN*VZgrfTOV zh2~mQlVWJ*_)2;3mR`~pZB{O};w6p2kqr}XtA@B5Wyt(tRMfkK$pq@(#x%kX80;+k z-BRo>>)!eo>A8#+xNN8xi1|(Q#e(68?~9oBWXzu)X5%LW$#YBW&lTw<{{D+Tr;S zRn8p7>*lrMll>;;sjlq`o>nTabKCrHbjpbb7C_{E{mtNOil*rG zEAJN5JY0C-KI$l$q=*W@NQBQ+3{gf&Hvr3AGj#V$^!&4q+J9RUd4*bg7V2C@#vzG566()qEje-dWkP1R^sx z#{STYl@D(?mvR9gAfl{Rf3$r>1%Ko#5IXj`S-sF%UBC6Apcj?F})7tYQ_GIdP!~R z_zi@%l6;b|trc`ILG2RoE!nJQ751EqM5v8tKC4dCoHC)Mx3irHi-Dww-pB=||Fce} z<_0g;*S{2%ZFa|(GjOqa8C}(vGlCw<)A5u032^g4gYnp~s}As0lj`0IvK(`L${a^1 zBPJB#ix$@?ia%H0;`s1i#|wZE*A;f9AAc*i(v5QAx8lq@n1mVM+M^}umlEQ$4kF(+ z7?(xrV5?JyeD>6DL<;z-OwztpzTTalVSvmfm}iP!w&qnV+98fo{}16KJDVF{^KfqU z0MK!Fgu^`zt{9#LP29#<6xsIdn;~Adzw9w(H2YSF7wxflpLE9GELYJ!Gp8cneXUHz zw}$$W?z+Nfe=lH>d`^%pIHJ)*5uRGz9s7{`DGE%kt=oa7Uc1(xxGcAU*{|PetVFfO zG^iaF`_9+tkb_@%sG-N=oQ5k$taNcyTGA_~ zMIi(F$|;xwUU&HWmw3Ywh*PxCJ2Y1tSws@drn3!hxn-S27v(qVs_R~<*KTzy70c4f zw*4($QI|+`uch1~A)&g9s#I4uCZi$3z3ofBFksvj>9ktUfUDE+8-)Af8y|bwRKA$C m`fZOA!N`d6{V4v*rrbX^RnmEC!T + + + Exe + net472 + latest + enable + PropertyChangedStorm + OwnNet.Audit.Runtime + x64 + x64 + + + + + + diff --git a/audit/runtime/README.md b/audit/runtime/README.md index 274cbdf7..d9a09086 100644 --- a/audit/runtime/README.md +++ b/audit/runtime/README.md @@ -8,9 +8,10 @@ a static finding clusters with it → **high confidence** (Plan.md §3.5). This layer covers the categories static analysis honestly can't (Plan.md §2): event/subscription & timer leaks confirmed under load (cat. 2/3), the -`DependencyPropertyDescriptor.AddValueChanged` leak (cat. 4), and the -**duplicated-immutable-data** detector — the project's "gold" (cat. 11). For these, -the runtime layer is the *only* tool, so they were `NO-TOOL` until now. +`DependencyPropertyDescriptor.AddValueChanged` leak (cat. 4), **PropertyChanged +storms** measured by raise-frequency (cat. 6), and the **duplicated-immutable-data** +detector — the project's "gold" (cat. 11). For these, the runtime layer is the *only* +tool, so they were `NO-TOOL` until now. ## Stack (Windows / build-required — Plan.md §4) @@ -40,6 +41,9 @@ audit/runtime/ DuplicateDetector/ # C# duplicate-immutable detector — Windows/build-required, NOT CI-gated DuplicateDetector.csproj # net472; Microsoft.Diagnostics.Runtime Program.cs # ClrMD over a full dump: group identical strings, wasted-bytes findings + PropertyChangedStorm/ # C# PropertyChanged-storm profiler — Windows/build-required, NOT CI-gated + PropertyChangedStorm.csproj # net472; Microsoft.Diagnostics.Tracing.TraceEvent + Program.cs # TraceEvent over an .etl: per-property raise frequency, storm findings ``` ## How the leak-harness works (Plan.md §4.1) @@ -88,11 +92,40 @@ python audit/runtime/ingest.py --duplicate-detector artifacts/own-audit/duplicat # -> run_static folds duplicate-detector.sarif in as a category-11 (P2) finding set. ``` +## PropertyChanged-storm profiler (Plan.md §2 cat. 6) + +Frequency — not correctness — is a runtime property. The static `INPC0xx` tier (cat. 5) +catches a missing `nameof` or a broken arg; it cannot see that `Total` fires +PropertyChanged 4 000x for one keystroke, half of them with **no value change**, +thrashing every binding. The profiler reads an ETW trace (`.etl`) captured while a +FlaUI scenario drove the target — a diagnostic build emits one event per raise via an +EventSource (`OwnNet-Sematix-INPC` / `Raised`, payload `{Type, Property, ValueChanged, +[SourceFile, SourceLine]}`) — aggregates per (type, property), and reports each +property over its per-operation threshold. When the build resolved a source file, a +storm clusters with a static `INPC0xx` hit in the same file → **high confidence** +(§3.5); otherwise it gets a unique `inpc:///` synthetic uri so distinct +storming properties stay distinct clusters. + +```bash +# on Windows, against an .etl captured during the scenario (PerfView / xperf / logman): +PropertyChangedStorm.exe --trace artifacts/own-audit/scenario.etl --operations 1 \ + --per-op-threshold 50 --out artifacts/own-audit/propertychanged-storm.json \ + --scenario open-declaration --target acme/LegacyApp --commit "$COMMIT" + +# then, anywhere (CI exercises this conversion): +python audit/runtime/ingest.py --propertychanged-storm \ + artifacts/own-audit/propertychanged-storm.json \ + --out artifacts/own-audit/propertychanged-storm.sarif +# -> run_static folds propertychanged-storm.sarif in as a category-6 (P2) finding set; +# a located storm clusters with a static INPC0xx in the same file. +``` + ## Selftest `ingest.py` carries embedded-fixture selftests (no harness, no Windows needed) and -gates on Linux CI — including the end-to-end check that a static OWN014 plus a -runtime leak in the same file form one high-confidence cluster: +gates on Linux CI — including the end-to-end checks that a static OWN014 plus a +runtime leak (and a static `INPC0xx` plus a runtime storm) in the same file each form +one high-confidence cluster: ```bash python audit/runtime/ingest.py --selftest @@ -100,10 +133,12 @@ python audit/runtime/ingest.py --selftest ## Status -- **Done:** the runtime→pipeline bridge (`ingest.py`, CI-gated, for both the - leak-harness and the duplicate detector), the leak-harness scenario schema + one - scenario, runtime rule mappings in the taxonomy (categories 2/3/4/11), the C# - leak-harness skeleton, and the C# duplicate-immutable detector (strings). +- **Done:** the runtime→pipeline bridge (`ingest.py`, CI-gated, for the leak-harness, + the duplicate detector and the PropertyChanged-storm profiler), the leak-harness + scenario schema + one scenario, runtime rule mappings in the taxonomy (categories + 2/3/4/6/11), the C# leak-harness skeleton, the C# duplicate-immutable detector + (strings), and the C# PropertyChanged-storm profiler (ETW). - **Deferred:** duplicate detection for arbitrary immutable types (field-by-field - content equality), the PropertyChanged-storm profiler (C# over ETW), - PerfView/SematixTrace wiring, and a scenario corpus for the top-N screens. + content equality), the diagnostic-build INPC `EventSource` instrumentation in the + target + PerfView/SematixTrace capture wiring, and a scenario corpus for the top-N + screens. diff --git a/audit/runtime/ingest.py b/audit/runtime/ingest.py index 222bc060..d10a8d7c 100644 --- a/audit/runtime/ingest.py +++ b/audit/runtime/ingest.py @@ -81,20 +81,21 @@ def _location(f: dict[str, Any]) -> tuple[str, int]: return (f.get("location") or f.get("type") or "runtime", int(f.get("line", 0) or 0)) -_DUP_SLUG_RE = re.compile(r"[^0-9A-Za-z._-]+") +_SLUG_RE = re.compile(r"[^0-9A-Za-z._-]+") -def _dup_uri(type_name: str, value: str, index: int) -> str: - """A UNIQUE synthetic uri for one heap-wide duplicate group. These findings have - no source line, so without a distinct uri every group would share the same - ``(basename, line)`` and the scorer would collapse Country, Currency, ... into a - single cluster — corrupting totals/heatmap and hiding distinct remediation items - (Codex review on #103). The ``index`` guarantees uniqueness even when two display - values share a prefix; the slug keeps the path readable. All groups roll up under - one ``heap://`` module, so the heatmap still buckets them together.""" - typ = (type_name or "immutable").replace("\\", ".").replace("/", ".") - slug = _DUP_SLUG_RE.sub("_", value or "").strip("_")[:40] or "value" - return f"heap://{typ}/{index:04d}-{slug}" +def _synthetic_uri(scheme: str, type_name: str, value: str, index: int) -> str: + """A UNIQUE synthetic uri for a runtime finding with no source line — a heap-wide + duplicate group (``heap://``), a storming property (``inpc://``). Without a + distinct uri every such finding would share the same ``(basename, line)`` and the + scorer would collapse Country/Currency or Total/Subtotal into a single cluster, + corrupting totals/heatmap and hiding distinct remediation items (Codex review on + #103). The ``index`` guarantees uniqueness even when two display values share a + prefix; the slug keeps the path readable. All findings of one ``scheme://`` + roll up under one heatmap module, so it still buckets them together.""" + typ = (type_name or "runtime").replace("\\", ".").replace("/", ".") + slug = _SLUG_RE.sub("_", value or "").strip("_")[:40] or "value" + return f"{scheme}://{typ}/{index:04d}-{slug}" def leak_harness_to_sarif(result: dict[str, Any]) -> dict[str, Any]: @@ -133,9 +134,10 @@ def duplicate_detector_to_sarif(result: dict[str, Any]) -> dict[str, Any]: if not f.get("report", True): continue # heap-wide duplicate groups have no source line; synthesize a UNIQUE uri per - # value so distinct groups stay distinct clusters (see _dup_uri). Always + # value so distinct groups stay distinct clusters (see _synthetic_uri). Always # file-level (line 0) -> no fabricated region. - uri = _dup_uri(str(f.get("type", "")), str(f.get("value", "")), len(findings)) + uri = _synthetic_uri("heap", str(f.get("type", "")), str(f.get("value", "")), + len(findings)) findings.append({ "rule": f.get("rule", "RUNTIME-DUP-IMMUTABLE"), "message": f.get("message", ""), @@ -151,6 +153,45 @@ def duplicate_detector_to_sarif(result: dict[str, Any]) -> dict[str, Any]: level="warning") +def propertychanged_storm_to_sarif(result: dict[str, Any]) -> dict[str, Any]: + """PropertyChanged-storm profiler JSON → SARIF (Plan.md §2 cat. 6, §4.3). The + profiler counts, over one user operation, how often each property raises + PropertyChanged and how many of those raises carry no value change (a missing + equality guard). A property over its per-operation threshold is a storm. When the + instrumentation resolved a source file the finding keeps it — so a storm clusters + with a static ``INPC0xx`` hit (cat. 5) in the same file → high confidence (§3.5); + otherwise we synthesize a unique per-property uri so distinct storming properties + stay distinct clusters. ``report: false`` (below threshold) is dropped; level + ``warning`` (a P2 perf finding, not a correctness error).""" + findings = [] + for f in result.get("findings", []): + if not f.get("report", True): + continue + loc = f.get("location") + if loc: + uri, line = str(loc), int(f.get("line", 0) or 0) + else: + # no resolved source line: unique per (type, property) so distinct + # storming properties don't collapse into one cluster. + uri = _synthetic_uri("inpc", str(f.get("type", "")), + str(f.get("property", "")), len(findings)) + line = 0 + findings.append({ + "rule": f.get("rule", "RUNTIME-PROPCHANGED-STORM"), + "message": f.get("message", ""), + "uri": uri, "line": line, + "properties": {k: f[k] for k in + ("type", "property", "raises", "redundantRaises", + "perOperation", "threshold") + if k in f}, + }) + return _runtime_sarif( + result.get("tool", "propertychanged-storm"), findings, + {"target": result.get("target", ""), "commit": result.get("commit", ""), + "scenario": result.get("scenario", ""), "operations": result.get("operations", 0)}, + level="warning") + + def main(argv: list[str] | None = None) -> int: ap = argparse.ArgumentParser(description="Convert a runtime tool's JSON result to SARIF.") # exactly one source tool — reject both so a stray flag fails fast instead of @@ -158,6 +199,7 @@ def main(argv: list[str] | None = None) -> int: src = ap.add_mutually_exclusive_group() src.add_argument("--leak-harness", help="leak-harness result JSON") src.add_argument("--duplicate-detector", help="duplicate-immutable-detector result JSON") + src.add_argument("--propertychanged-storm", help="PropertyChanged-storm profiler result JSON") ap.add_argument("--out", default="", help="output SARIF (defaults per tool under artifacts/)") ap.add_argument("--selftest", action="store_true") args = ap.parse_args(argv) @@ -172,8 +214,13 @@ def main(argv: list[str] | None = None) -> int: result = json.loads(Path(args.duplicate_detector).read_text(encoding="utf-8")) sarif = duplicate_detector_to_sarif(result) default_out = "artifacts/own-audit/duplicate-detector.sarif" + elif args.propertychanged_storm: + result = json.loads(Path(args.propertychanged_storm).read_text(encoding="utf-8")) + sarif = propertychanged_storm_to_sarif(result) + default_out = "artifacts/own-audit/propertychanged-storm.sarif" else: - ap.error("one of --leak-harness / --duplicate-detector is required (or --selftest)") + ap.error("one of --leak-harness / --duplicate-detector / " + "--propertychanged-storm is required (or --selftest)") out = Path(args.out or default_out) out.parent.mkdir(parents=True, exist_ok=True) @@ -302,6 +349,60 @@ def check(ok: bool, msg: str) -> None: check(dscored["by_category"].get("duplicate-immutable") == 2, f"both duplicate values must count under category 11, got {dscored['by_category']}") + # PropertyChanged-storm profiler (cat. 6): per-operation raise frequency. A storm + # with a resolved source file keeps its line (so it clusters with a static INPC0xx + # hit in the same file); storms with no source line get unique per-property uris so + # distinct properties stay distinct clusters; below-threshold is dropped. + storm = propertychanged_storm_to_sarif({ + "tool": "propertychanged-storm", "target": "acme/LegacyApp", + "scenario": "open-declaration", "operations": 1, + "findings": [ + {"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel", + "property": "Total", "location": "src/Vm/DeclarationViewModel.cs", "line": 88, + "raises": 4200, "redundantRaises": 3990, "perOperation": 4200.0, + "threshold": 50, "report": True, + "message": "Total raised PropertyChanged 4200x/op (3990 with no value change)"}, + {"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel", + "property": "Subtotal", "raises": 900, "redundantRaises": 880, + "perOperation": 900.0, "threshold": 50, "report": True, + "message": "Subtotal raised PropertyChanged 900x/op (880 with no value change)"}, + {"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel", + "property": "Title", "raises": 2, "redundantRaises": 0, "perOperation": 2.0, + "threshold": 50, "report": False, "message": "below threshold"}, + ]}) + sres = storm["runs"][0]["results"] + check(len(sres) == 2, f"below-threshold storm must be dropped: got {len(sres)}") + check(all(r["level"] == "warning" for r in sres), + "propertychanged-storm must be SARIF level warning (P2)") + located = [r for r in sres + if r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] + == "src/Vm/DeclarationViewModel.cs"] + check(len(located) == 1 + and located[0]["locations"][0]["physicalLocation"].get("region", {}) + .get("startLine") == 88, + "a storm with a resolved source file must keep its source line") + suris = {r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] for r in sres} + check(len(suris) == 2, f"distinct storming properties need distinct uris, got {suris}") + snorm = normalize_results(parse_sarif(json.dumps(storm), "propertychanged-storm", []), tax) + scat = {f.rule: (f.category, f.category_name) for f in snorm} + check(scat.get("RUNTIME-PROPCHANGED-STORM") == (6, "propertychanged-storm"), + f"propertychanged-storm -> category 6, got {scat.get('RUNTIME-PROPCHANGED-STORM')}") + # Plan §3.5: the located storm + a static INPC0xx in the same file -> high confidence. + inpc_sarif = json.dumps({"version": "2.1.0", "runs": [{ + "tool": {"driver": {"name": "Own.NET"}}, "results": [ + {"ruleId": "INPC003", "level": "warning", + "message": {"text": "raise PropertyChanged without an equality check"}, + "locations": [{"physicalLocation": { + "artifactLocation": {"uri": "src/Vm/DeclarationViewModel.cs"}, + "region": {"startLine": 87}}}]}]}]}) + both_s = normalize_results(parse_sarif(inpc_sarif, "own-check", []), tax) + snorm + sscored = score(both_s, tax, line_tol=3) + confirmed_s = [c for c in sscored["clusters"] if c.confidence == "high" + and set(c.tools) == {"own-check", "propertychanged-storm"}] + check(len(confirmed_s) == 1, + f"static INPC0xx + runtime storm in the same file must form 1 high-confidence " + f"cluster, got {[(c.module, c.tools, c.confidence) for c in sscored['clusters']]}") + fails = [c for c in checks if c] for f in fails: print(f"INGEST SELFTEST FAIL: {f}") diff --git a/audit/static/run_static.py b/audit/static/run_static.py index 555d5ea2..f9a80949 100755 --- a/audit/static/run_static.py +++ b/audit/static/run_static.py @@ -146,7 +146,8 @@ def run(target: str, profile: dict[str, Any], out_dir: Path, target_name: str = # with its static OWN014/OWN001 -> high confidence (§3.5) through the orchestrator, # not only the lower-level aggregate(). for fname, tool in (("leak-harness.sarif", "leak-harness"), - ("duplicate-detector.sarif", "duplicate-detector")): + ("duplicate-detector.sarif", "duplicate-detector"), + ("propertychanged-storm.sarif", "propertychanged-storm")): rt = out_dir / fname if rt.exists(): sarif_inputs.append((tool, str(rt))) @@ -284,6 +285,13 @@ def check(ok: bool, msg: str) -> None: # total derives from the call count "locations": [{"physicalLocation": {"artifactLocation": { "uri": "heap://System.String/0000-Country"}}}]}]}]} (out2 / "duplicate-detector.sarif").write_text(json.dumps(dup), encoding="utf-8") + # a runtime propertychanged-storm SARIF must be folded in by the same loop. + storm = {"runs": [{"tool": {"driver": {"name": "propertychanged-storm"}}, "results": [ + {"ruleId": "RUNTIME-PROPCHANGED-STORM", "level": "warning", + "message": {"text": "Total raised PropertyChanged 4200x/op"}, + "locations": [{"physicalLocation": {"artifactLocation": { + "uri": "inpc://Acme.Vm.DeclarationViewModel/0000-Total"}}}]}]}]} + (out2 / "propertychanged-storm.sarif").write_text(json.dumps(storm), encoding="utf-8") profile = {"name": "t", "severity_floor": "warning", "tiers": {"build_free": []}} res2 = run("/nonexistent-target", profile, out2, target_name="t/p") check(any(t["tool"] == "roslyn-pack" for t in res2["tiers"]), @@ -292,6 +300,8 @@ def check(ok: bool, msg: str) -> None: # total derives from the call count "runtime leak-harness.sarif must be folded in by run()") check(any(t["tool"] == "duplicate-detector" for t in res2["tiers"]), "runtime duplicate-detector.sarif must be folded in by run()") + check(any(t["tool"] == "propertychanged-storm" for t in res2["tiers"]), + "runtime propertychanged-storm.sarif must be folded in by run()") check(res2["totals"]["high_confidence"] >= 1, "runtime leak + static finding in one file must form a high-confidence cluster") diff --git a/audit/static/taxonomy/categories.yml b/audit/static/taxonomy/categories.yml index 4190f5b6..380b804f 100644 --- a/audit/static/taxonomy/categories.yml +++ b/audit/static/taxonomy/categories.yml @@ -64,6 +64,10 @@ rules: "RUNTIME-LEAK-TIMER": {category: 3, name: timer-leak} "RUNTIME-DPD-ADDVALUECHANGED": {category: 4, name: dpd-addvaluechanged-leak} "RUNTIME-DUP-IMMUTABLE": {category: 11, name: duplicate-immutable} + "RUNTIME-PROPCHANGED-STORM": {category: 6, name: propertychanged-storm} # raise-frequency, + # NOT INPC0xx correctness (cat. 5) — a storm + # in the same file as an INPC0xx hit clusters + # with it -> high confidence (§3.5) # Baseline P-level per category (Plan.md §3.5.2). Confidence (cross-tool # agreement) is a SEPARATE axis handled by score.py; this is severity only. @@ -73,6 +77,7 @@ category_severity: 3: P1 4: P1 # DependencyPropertyDescriptor.AddValueChanged leak (runtime-confirmed) 5: P2 + 6: P2 # PropertyChanged storms/cascades — runtime raise-frequency, perf-tier 9: P2 11: P2 # duplicated immutable data — memory bloat (the project's "gold"), perf-tier 14: P2 From 28f29b88703ec2025508383afd35d59ee7160dfc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 07:21:25 +0000 Subject: [PATCH 2/3] audit: storm finding uses source location only when line>=1 (PR #104 review) Address Codex: propertychanged_storm_to_sarif wrote the bare source file at line 0 whenever the profiler resolved a SourceFile but not a SourceLine (the C# falls back to 0). The scorer clusters by basename + line window, so every file-only storm in one file collapsed into a single cluster, and a static finding on lines 1-3 could become a false high-confidence match. A source location is now usable only when line >= 1 (the same discipline as render_sarif's "region only when line>=1"); otherwise the finding keeps its unique per-property inpc:// synthetic uri. Selftest adds a file-only (line 0) storm and asserts it falls back to a synthetic uri with no region. ingest selftest 25/25; run_static 17/17; ruff clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE --- audit/runtime/ingest.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/audit/runtime/ingest.py b/audit/runtime/ingest.py index d10a8d7c..03d641da 100644 --- a/audit/runtime/ingest.py +++ b/audit/runtime/ingest.py @@ -167,12 +167,16 @@ def propertychanged_storm_to_sarif(result: dict[str, Any]) -> dict[str, Any]: for f in result.get("findings", []): if not f.get("report", True): continue + # A source location is usable for clustering ONLY when the line is resolved + # (>= 1). The C# falls back to line 0 when only the file is known; writing the + # bare file at line 0 would collapse every file-only storm into one cluster and + # could false-match a static finding on lines 1-3 (Codex review on #104). In + # that case keep a unique per-property synthetic uri instead. loc = f.get("location") - if loc: - uri, line = str(loc), int(f.get("line", 0) or 0) + line = int(f.get("line", 0) or 0) + if loc and line >= 1: + uri = str(loc) else: - # no resolved source line: unique per (type, property) so distinct - # storming properties don't collapse into one cluster. uri = _synthetic_uri("inpc", str(f.get("type", "")), str(f.get("property", "")), len(findings)) line = 0 @@ -366,12 +370,19 @@ def check(ok: bool, msg: str) -> None: "property": "Subtotal", "raises": 900, "redundantRaises": 880, "perOperation": 900.0, "threshold": 50, "report": True, "message": "Subtotal raised PropertyChanged 900x/op (880 with no value change)"}, + # file resolved but LINE unresolved (C# falls back to 0): must NOT use the + # bare file uri — keep a synthetic uri so it can't collapse/false-match. + {"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel", + "property": "Discount", "location": "src/Vm/DeclarationViewModel.cs", "line": 0, + "raises": 700, "redundantRaises": 690, "perOperation": 700.0, + "threshold": 50, "report": True, + "message": "Discount raised PropertyChanged 700x/op (file known, line unresolved)"}, {"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel", "property": "Title", "raises": 2, "redundantRaises": 0, "perOperation": 2.0, "threshold": 50, "report": False, "message": "below threshold"}, ]}) sres = storm["runs"][0]["results"] - check(len(sres) == 2, f"below-threshold storm must be dropped: got {len(sres)}") + check(len(sres) == 3, f"below-threshold storm must be dropped: got {len(sres)}") check(all(r["level"] == "warning" for r in sres), "propertychanged-storm must be SARIF level warning (P2)") located = [r for r in sres @@ -380,9 +391,16 @@ def check(ok: bool, msg: str) -> None: check(len(located) == 1 and located[0]["locations"][0]["physicalLocation"].get("region", {}) .get("startLine") == 88, - "a storm with a resolved source file must keep its source line") + "only the line-resolved storm keeps the bare source file (line>=1)") + # the file-only (line 0) storm falls back to a synthetic inpc:// uri, no region. + file_only = [r for r in sres if "Discount" in r["message"]["text"]] + check(len(file_only) == 1 + and file_only[0]["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] + .startswith("inpc://") + and "region" not in file_only[0]["locations"][0]["physicalLocation"], + "a file-only storm (line 0) must fall back to a synthetic inpc:// uri") suris = {r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] for r in sres} - check(len(suris) == 2, f"distinct storming properties need distinct uris, got {suris}") + check(len(suris) == 3, f"distinct storming properties need distinct uris, got {suris}") snorm = normalize_results(parse_sarif(json.dumps(storm), "propertychanged-storm", []), tax) scat = {f.rule: (f.category, f.category_name) for f in snorm} check(scat.get("RUNTIME-PROPCHANGED-STORM") == (6, "propertychanged-storm"), From be9c48212567d501f051bf3e0758aa4dcee4c17c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 07:25:08 +0000 Subject: [PATCH 3/3] audit: strengthen storm selftest + fix README uri format (PR #104 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two CodeRabbit nitpicks: - ingest.py selftest: assert the two UNRESOLVED storms (Subtotal: no location, Discount: file-only line 0) get DISTINCT synthetic inpc:// uris, not just that the total uri count is 3 — so a regression that reused one synthetic uri for every unresolved property is caught directly. - README: the synthetic uri is inpc:///- (indexed), not inpc:///; correct the docs to match what ingest.py emits. ingest selftest 26/26; run_static 17/17; ruff clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QDPpNT9Uh8RoTrKPvgcoRE --- audit/runtime/README.md | 5 +++-- audit/runtime/ingest.py | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/audit/runtime/README.md b/audit/runtime/README.md index d9a09086..cb4ebb9f 100644 --- a/audit/runtime/README.md +++ b/audit/runtime/README.md @@ -103,8 +103,9 @@ EventSource (`OwnNet-Sematix-INPC` / `Raised`, payload `{Type, Property, ValueCh [SourceFile, SourceLine]}`) — aggregates per (type, property), and reports each property over its per-operation threshold. When the build resolved a source file, a storm clusters with a static `INPC0xx` hit in the same file → **high confidence** -(§3.5); otherwise it gets a unique `inpc:///` synthetic uri so distinct -storming properties stay distinct clusters. +(§3.5); otherwise (file-only with no line, or no location at all) it gets a unique +`inpc:///-` synthetic uri — the `` index keeps distinct +storming properties in distinct clusters even when their slugs collide. ```bash # on Windows, against an .etl captured during the scenario (PerfView / xperf / logman): diff --git a/audit/runtime/ingest.py b/audit/runtime/ingest.py index 03d641da..daa7906c 100644 --- a/audit/runtime/ingest.py +++ b/audit/runtime/ingest.py @@ -401,6 +401,13 @@ def check(ok: bool, msg: str) -> None: "a file-only storm (line 0) must fall back to a synthetic inpc:// uri") suris = {r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] for r in sres} check(len(suris) == 3, f"distinct storming properties need distinct uris, got {suris}") + # the TWO unresolved storms (Subtotal: no location, Discount: file-only line 0) + # must get DISTINCT synthetic uris — not collapse onto one (CodeRabbit review #104). + synthetic = [r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] for r in sres + if r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] + .startswith("inpc://")] + check(len(synthetic) == 2 and len(set(synthetic)) == 2, + f"unresolved storming properties need distinct synthetic uris, got {synthetic}") snorm = normalize_results(parse_sarif(json.dumps(storm), "propertychanged-storm", []), tax) scat = {f.rule: (f.category, f.category_name) for f in snorm} check(scat.get("RUNTIME-PROPCHANGED-STORM") == (6, "propertychanged-storm"),