Proposal: Structured triage and provenance-aware component analysis for ADR detection #50

Activity

1. pengyuzhang commented on Aug 21, 2026

   

   pengyuzhang

   on Aug 21, 2026

   Collaborator

   More actions

   Thanks for writing this up before opening PRs — that's the right order, and the problem statement is concrete enough to respond to point by point. Here's where I land on each of the three pieces, plus one issue with the evidence.

   Import-following provenance — accept. This is the one I'd take. It's a verified coverage hole, not a design preference: get_source_code hands Tier 2 a 21-line wrapper while the actual behavior sits in function/. access_auditor is the clearest example — the registered path is a FastMCP wrapper whose body is return audit_user_permissions(...), and the excessive-permission-scope logic is all in function/access_auditor.py, which Tier 2 never sees. Fixing it is bugfix-scale, touches one file, and doesn't change any output semantics.

   One condition: a control run with docstrings and comments stripped. Two of the six vulnerable split-file servers describe their own vulnerability in prose (function/access_auditor.py says "excessive permission scope ... violating the principle of least privilege"), so some of the recall gain from widening file coverage could be label leakage rather than better reasoning. The stripped run separates those.

   Structured triage contract — accept conditionally. Behind a config flag, default off, stock text contract preserved, fail-toward-escalate preserved. Low blast radius, and it hardens a parse path that has already needed one fix (your PR #44). Reasonable.

   Dual-axis threat output — defer. It conflicts with how both benchmarks label ground truth, it makes published metrics ambiguous unless the selected axis is recorded in the analysis output, and the only evidence for it is the non-public provenance-flow dataset. This is the largest change and the least supported of the three.

   On the evidence: the numbers argue against the framing. Only 2 of the 42 malicious tasks (7 and 8) use the six split-file servers, so the provenance fix — the part worth landing — can account for at most ~2 of the +7 TP. The other ~5 came from the triage contract and the axis-split semantics, i.e. from prompt and decision-rule changes rather than from provenance. The delta is presented bundled and attributed to the architecture story; without a per-component ablation, the headline is unsupported even taken at face value. The existing enable_threat_intelligence / enable_source_code / enable_policy toggles in config_detector.yaml should make that ablation straightforward.
2. shengqi-gensee commented on Aug 25, 2026

   

   shengqi-gensee

   on Aug 25, 2026

   ContributorAuthor

   More actions

   Thanks for the detailed response. We agree with the direction you proposed: keep the changes optional and independently reviewable, separate the narrow import-following change from the structured triage contract, and defer the broader reasoning changes.

   I would like to clarify several terms in the original issue to avoid any confusions. We use the following definitions:

   • D refers to the dual-axis threat output already described in the original proposal.
   • S refers to structured Tier-1 contract which includes the schema and the routing logic that consumes it.
   • P was used more broadly in the original proposal than in your response: it includes both requested-and-audited provenance investigation (P1) and bounded import-following source retrieval (P2). The import-following change you identified corresponds to P2.

   We ran the following build-up on the frozen 303-task ADR-Bench cohort using GPT-5.6 Luna low for both tiers:

   Configuration	TP	FN	FP	TN	Recall	FPR	Accuracy
   Original ADR	31	11	36	225	73.8%	13.8%	84.5%
   + D	36	6	22	239	85.7%	8.4%	90.8%
   + D + S	37	5	22	239	88.1%	8.4%	91.1%
   + D + S + P1	38	4	32	229	90.5%	12.3%	88.1%
   + D + S + P1 + P2	39	3	30	231	92.9%	11.5%	89.1%

   The rows use the same frozen cohort, Tier-1 checkpoint set, detector snapshot, model configuration, and scoring contract. The final two rows come from a focused paired provenance-content follow-up. Note that an earlier repeat of the complete profile produced 95.2% recall and 10.7% FPR; the refreshed complete-profile row above produced 92.9% recall and 11.5% FPR. The prompts, tools, and frozen inputs for those complete-profile calls matched, so we treat that difference as luna-model variance.

   To keep review scope small, we propose the following order:

   1. Model configurability. Add an optional profile using Luna for both Tier 1 and Tier 2 while preserving the existing default models and detector behavior.

   2. P2: import-following source retrieval. Submit the bounded import-following provider change independently, with focused split-file, traversal-bound, missing-import, and comments/docstrings-stripped tests.

   3. S: structured triage contract. Submit the structured route as a separate configuration option, default off, preserving the stock text contract when disabled and failing toward escalation on invalid structured output.

   We would not include P1 in the import-following PR. P1 is a broader architecture change involving requested and audited provenance calls, so we can discuss its interface and scope separately after the narrower P2 and S changes have been reviewed.

   D is included above to clarify how the previously reported result was built, not to propose bundling the dual-axis reasoning change into these initial PRs.

   Does this separation and submission order align with what you had in mind?
3. pengyuzhang commented on Aug 28, 2026

   

   pengyuzhang

   on Aug 28, 2026

   Collaborator

   More actions

   Thanks—the separation is broadly aligned, with a few changes.

   P2 is approved as the first PR. Treat it as a narrow source-coverage bug fix. Import discovery should be static and bounded: do not execute modules; enforce resolved-path and symlink containment; limit recursion depth, file count, and total bytes; deduplicate cycles; preserve deterministic ordering; and report incomplete or unavailable coverage explicitly.

   Please include both focused unit tests and the previously requested benchmark control with comments and docstrings removed. A stripped-source unit test alone does not establish whether the measured recall improvement came from implementation behavior or vulnerability-describing prose.

   S can follow as a separate PR. It should remain default off, preserve the existing text contract exactly when disabled, and escalate on malformed, missing, or internally inconsistent structured output.

   Model configurability should be separate architecture work, not a prerequisite for P2. Tier 2 currently invokes the Claude CLI directly. Supporting gpt-5.6-luna therefore requires a new reasoning backend and MCP/tool-calling path, not merely another YAML profile. A proposal should specify the provider/API, exact model identifier, reasoning effort, tool interface, authentication requirements, and how output and usage data remain comparable across backends.

   The new build-up table is helpful, but it is not yet an independent ablation. S is measured only with D enabled, and P2 only with D+S+P1 enabled. Please provide P2-only versus original, or full-minus-P2, along with exact task-level prediction changes and repeated paired runs. P2’s observed +1 TP/−2 FP improvement is exactly the same magnitude as the complete-profile variance you reported, so it should not yet be presented as a reliable performance gain. This does not block the verified coverage fix.

   D and P1 remain deferred. P1 currently adds ten false positives and reduces accuracy. Both changes need independent design and evaluation before reconsideration.

   Recommended order:

   1. P2 source-coverage fix.
   2. S structured triage option.
   3. Separate provider/backend proposal if broader model support is desired.

   With that scope, please proceed with the P2 PR.