ADR-0001: Real ML detection and honest benchmarks - #1
Merged
nicholas-ruest merged 1 commit intoJul 27, 2026
Merged
Conversation
…aims
ADR-0001 Phase 1 (truthfulness). The audit-trail fix is the load-bearing part;
Phases 2 (real benchmarks) and 3 (ML wiring) are not attempted here.
PromptInjection:
- `detection_method` is now set by the branch that executed ("heuristic" /
"onnx-deberta"), never derived from `config.use_fallback`. Emitted on the
passing branch too, which previously carried no provenance at all.
- New `detection_mode_degraded` metadata plus a `tracing::warn!` whenever ML was
requested and heuristics ran instead. Silent substitution is now impossible.
- Control flow un-inverted: ML is attempted first whenever `model_path` is set,
instead of `use_fallback: true` short-circuiting past it.
- `use_fallback: false` with no model returns Err rather than falling through.
- `detect_ml` is no longer `#[allow(dead_code)]`; it is reachable and fails
honestly. Doc comments no longer describe the scanner as ML-based.
Correction to the ADR's premise: the literal `detection_method: "ml"` was in
fact unreachable pre-fix. The `Err(_) if use_fallback` arm sat inside the `else`
of `if use_fallback`, so its guard could never hold and `use_fallback: false`
always returned Err before reaching the metadata line. The config echo was a
latent defect that would have gone live the moment `detect_ml` was implemented,
not an active falsification. Test comments record this accurately.
Tests (ADR Verification §1, §2):
- tests/detection_provenance_audit.rs runs all 1000 prompts from
benchmarks/data/test_prompts.json through the compiled `Scanner::scan` API in
three configurations, asserting provenance on every result. 2 of its 3 tests
fail against the pre-fix code.
- Four unit tests pinning the provenance contract.
Unblocking: the llm-shield-scanners test target did not compile on main (11
errors in output/{factuality,reading_time,sensitive}.rs test modules — stale
field name, &str/String, `.parse()` on serde_json::Value). Fixed mechanically so
the ADR's verification could be executed at all. This exposed 23 pre-existing
test failures in unrelated scanners, left unfixed and reported in the PR.
Docs: withdrew the performance comparison table and the "all claims validated"
line, marked ML/NER features ROADMAP — NOT YET ACTIVE, added a Detection Methods
section, banner-marked the four simulated benchmark reports SIMULATED — NOT
MEASURED, and renamed bench_latency_runner.py to simulate_latency_benchmark.py.
Implements shield/docs/adr/ADR-0001-real-ml-detection-and-honest-benchmarks.md
nicholas-ruest
deleted the
adr/0001-real-ml-detection-and-honest-benchmarks
branch
July 27, 2026 20:26
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
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.
Implements Phase 1 of
docs/adr/ADR-0001-real-ml-detection-and-honest-benchmarks.md. Phases 2 (real benchmarks) and 3 (ML wiring) are not attempted — see Left undone for why.The audit-trail fix (the non-negotiable part)
PromptInjectionnow reports what actually executed:detection_methodis set by the branch that ran ("heuristic"/"onnx-deberta"), never derived fromconfig.use_fallback. It is also emitted on the passing branch, which previously carried no provenance at all — a benign verdict was completely unattributable.detection_mode_degradedflag +tracing::warn!whenever ML was requested and heuristics ran instead. Per ADR §3, degradation is now loud rather than silent.model_pathis set, instead ofuse_fallback: true(the default) short-circuiting past it under a comment reading "Try ML detection first".use_fallback: falsewith no model returnsErrrather than falling through to heuristics.detect_mlis no longer#[allow(dead_code)]— it is reachable and fails honestly.Correction to the ADR's premise (please read)
The ADR states at §1 that setting
use_fallback: falsemakesdetect_mlfail, fall through todetect_heuristic, and still stamp the resultdetection_method: "ml". I could not reproduce that, and on inspection it cannot happen. The pre-fix match was:The fallback arm's guard is
use_fallback, but it sits inside theelseofif use_fallback— so it is unreachable, anduse_fallback: falsealways returnedErrbefore reaching the metadata line. The literal"ml"was therefore never emitted by the shipped code.This does not make the fix unnecessary, but it changes its character: the config echo was a latent defect that would have gone live the instant
detect_mlwas implemented in Phase 3 (step 16), not an active falsification in production today. I have recorded this accurately in the test comments rather than letting a test claim to pin a bug it does not pin. ADR §1 (line 68) and the Risks paragraph (line 149) should be amended.What was live pre-fix and is genuinely fixed: ML unreachable under default config; no provenance on passing results; no signal at all for a requested-but-skipped ML run.
Test evidence
tests/detection_provenance_audit.rs(ADR Verification §1) runs all 1000 prompts frombenchmarks/data/test_prompts.jsonthrough the real compiledScanner::scanAPI in three configurations — 3000 live scans — asserting provenance on every result.Confirmed these fail against the pre-fix code. I restored the original
prompt_injection.rsunder the new tests and re-ran:(The third passes pre-fix for the unreachable-arm reason explained above; it is kept as a regression pin and its comment says so.)
1. The
llm-shield-scannerstest target did not compile onmain. 11 errors, all in#[cfg(test)]blocks ofoutput/{factuality,reading_time,sensitive}.rs: a renamed field (sanitized_input→sanitized_text),&strvsString, and.parse()called on aserde_json::Value. No test in this crate could run. I fixed them mechanically because otherwise the ADR's Verification section is unexecutable. Flagging it because it means this crate's suite has been silently dead in CI.2. That unblocking exposed 23 pre-existing test failures in scanners I did not touch —
gibberish(7),secrets(7),sensitive(3),relevance(2),language(2),invisible_text(1),malicious_urls(1). These are real product defects, not artifacts of this PR. Some look security-relevant, e.g.test_sensitive_credit_card_detectionfails onassert!(!result.is_valid)— the scanner does not flag a credit card. Left unfixed and out of scope; they need their own issue.cargo test -p llm-shield-scannersis red on this branch, and was red-by-non-compilation before.Separately,
llm-shield-sdkfails to compile onmain(arity mismatch, 4 errors) — also pre-existing and untouched.Documentation changes
I made the README edits enumerated in ADR §5 rather than only describing them, since the ADR specifies them line by line. Each one:
:9:19:25ROADMAP — NOT YET ACTIVE:27ROADMAP — NOT YET ACTIVE:40-52re.searchloop ÷ hardcoded baseline; throughput a generator input constant):43-44:986ESTIMATE — NOT MEASURED:1123detection_method/detection_mode_degraded, and an explicit statement of what heuristics do and do not catchAlso (ADR §6, §7):
SIMULATED — NOT MEASUREDbanners onLATENCY_BENCHMARK_REPORT.md,THROUGHPUT_ANALYSIS_REPORT.md,THROUGHPUT_DELIVERABLES.md,docs/archive/THROUGHPUT_BENCHMARK_COMPLETE.md; "CLAIM VALIDATED"/"VALIDATED" corrected in the two throughput docs;bench_latency_runner.py→simulate_latency_benchmark.pywith its one live reference inbench_latency.shupdated.Marketing copy is a judgment call — if any wording above overshoots or undershoots, it is easy to adjust and I would rather it be reviewed than assumed.
Left undone
Phase 3 (ML wiring) — deliberately not started. I verified
llm-shield-modelscompiles cleanly here, so the Cargo dependency edge (gap (a)) is technically addable. I did not add it, because the ADR sequences step 13 together with steps 14–18 and gates them on Verification §3–§5, and those cannot be satisfied in this environment:models/registry.jsoncannot be honestly repaired. Fixing it requires genuine SHA-256 hashes of real artifacts. Substituting plausible-looking hex would be worse than the current obviously-invalid placeholders: a non-hex placeholder fails loudly, a wrong-but-well-formed hash fails confusingly and looks like a verified artifact.detect_mlwould ship untested tensor-marshalling code in a security scanner and would break any user who supplies real weights.So
detect_mlfails with a clear message naming the missing dependency, and no code path can report ML. Whether to ship a partial ML wiring is a product decision, not mine to make.Phase 2 (real benchmarks) — not started. Requires disclosed physical hardware and an executed llm-guard baseline. No numbers were generated, and none were invented; the claims are withdrawn rather than replaced.
Phase 4 (CI guardrails) — not started.
Corpus shortfall. ADR Verification §1 asks for ≥500 positive / ≥500 benign.
test_prompts.jsonprovides 1000 prompts but only 100 labeledinjection. Benign coverage is met (600+); the positive set is not. I used the real corpus as-is rather than synthesizing 400 additional positives. Accuracy/precision/recall (§6) and adversarial robustness (§7) are consequently not measured here.toxicity.rs/sentiment.rs— ADR §2 asks to apply the same fix to these. They carry the sameuse_fallbackconfig field, but neither has adetect_mlnor emitsdetection_methodat all, so neither can misreport. Theiruse_fallbackfield is simply unread. Left alone to keep this PR to the actual defect; worth a follow-up to remove the dead config field.