Skip to content

ADR-0001: Real ML detection and honest benchmarks - #1

Merged
nicholas-ruest merged 1 commit into
mainfrom
adr/0001-real-ml-detection-and-honest-benchmarks
Jul 27, 2026
Merged

ADR-0001: Real ML detection and honest benchmarks#1
nicholas-ruest merged 1 commit into
mainfrom
adr/0001-real-ml-detection-and-honest-benchmarks

Conversation

@nicholas-ruest

Copy link
Copy Markdown
Member

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)

PromptInjection now reports what actually executed:

  • detection_method is set by the branch that ran ("heuristic" / "onnx-deberta"), never derived from config.use_fallback. It is also emitted on the passing branch, which previously carried no provenance at all — a benign verdict was completely unattributable.
  • New detection_mode_degraded flag + tracing::warn! whenever ML was requested and heuristics ran instead. Per ADR §3, degradation is now loud rather than silent.
  • Control flow un-inverted. ML is attempted first whenever model_path is set, instead of use_fallback: true (the default) short-circuiting past it under a comment reading "Try ML detection first".
  • The strict case is strict. use_fallback: false with no model returns Err rather than falling through to heuristics.
  • detect_ml is 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: false makes detect_ml fail, fall through to detect_heuristic, and still stamp the result detection_method: "ml". I could not reproduce that, and on inspection it cannot happen. The pre-fix match was:

let (score, indicators) = if self.config.use_fallback {
    self.detect_heuristic(input)
} else {
    match self.detect_ml(input) {
        Ok(result) => result,
        Err(_) if self.config.use_fallback => self.detect_heuristic(input),  // guard can never hold
        Err(e) => return Err(e),                                            // always taken
    }
};

The fallback arm's guard is use_fallback, but it sits inside the else of if use_fallback — so it is unreachable, and use_fallback: false always returned Err before 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_ml was 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 from benchmarks/data/test_prompts.json through the real compiled Scanner::scan API in three configurations — 3000 live scans — asserting provenance on every result.

########## UNIT: prompt_injection ##########
running 14 tests
test input::prompt_injection::tests::test_ml_configured_but_unavailable_degrades_and_reports_heuristic ... ok
test input::prompt_injection::tests::test_detection_method_is_heuristic_in_default_config ... ok
test input::prompt_injection::tests::test_no_config_yields_bare_ml_detection_method ... ok
test input::prompt_injection::tests::test_ml_required_without_model_errors_instead_of_reporting_ml ... ok
[... 10 pre-existing tests ...]
test result: ok. 14 passed; 0 failed; 0 ignored; 0 measured; 218 filtered out; finished in 0.03s

########## INTEGRATION: corpus provenance audit ##########
running 3 tests
[ml-required, model absent] n=1000 errored=1000
test ml_required_with_model_absent_errors_over_full_corpus ... ok
[ml-requested, unavailable] n=1000 degraded=1000
test ml_unavailable_with_fallback_reports_heuristic_and_degraded ... ok
[heuristic mode] n=1000 methods={"heuristic": 1000}
[heuristic mode] by category: {"code": {"heuristic": 100}, "injection": {"heuristic": 100}, "long": {"heuristic": 200}, "medium": {"heuristic": 200}, "secrets": {"heuristic": 100}, "simple": {"heuristic": 200}, "toxic": {"heuristic": 100}}
test heuristic_mode_never_reports_ml_over_full_corpus ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.14s

Confirmed these fail against the pre-fix code. I restored the original prompt_injection.rs under the new tests and re-ran:

running 3 tests
test ml_required_with_model_absent_errors_over_full_corpus ... ok
test ml_unavailable_with_fallback_reports_heuristic_and_degraded ... FAILED
test heuristic_mode_never_reports_ml_over_full_corpus ... FAILED
test result: FAILED. 1 passed; 2 failed

(The third passes pre-fix for the unreachable-arm reason explained above; it is kept as a regression pin and its comment says so.)

⚠️ Two things reviewers should know

1. The llm-shield-scanners test target did not compile on main. 11 errors, all in #[cfg(test)] blocks of output/{factuality,reading_time,sensitive}.rs: a renamed field (sanitized_inputsanitized_text), &str vs String, and .parse() called on a serde_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_detection fails on assert!(!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-scanners is red on this branch, and was red-by-non-compilation before.

Separately, llm-shield-sdk fails to compile on main (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:

ADR ref Change
:9 Removed "delivering 10x faster"; added a withdrawal notice linking the ADR
:19 "⚡ 10x Performance" → "Native Rust Scanning", speedup noted as unmeasured
:25 ML-Ready now says the ONNX integration is a library with no scanner wired to it; ROADMAP — NOT YET ACTIVE
:27 Removed "(95-99% accuracy)"; DeBERTa-v3 PII marked ROADMAP — NOT YET ACTIVE
:40-52 Deleted the performance table rather than relabeling — I have no measured values to put in it. Replaced with a plain statement of what the numbers actually were (Python re.search loop ÷ hardcoded baseline; throughput a generator input constant)
:43-44 Removed "🎯 All performance claims validated"
:986 Per-scanner latency/throughput/memory/binary lists collapsed and marked ESTIMATE — NOT MEASURED
:1123 "Validated 10-100x improvement ... (23,815x)" → "Not yet measured"
new Added a Detection Methods section: per-scanner backend table, how to read detection_method / detection_mode_degraded, and an explicit statement of what heuristics do and do not catch

Also (ADR §6, §7): SIMULATED — NOT MEASURED banners on LATENCY_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.pysimulate_latency_benchmark.py with its one live reference in bench_latency.sh updated.

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-models compiles 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:

  • No model weights exist in the repo and none can be fetched and checksum-verified here (ADR Verification §4, §5).
  • models/registry.json cannot 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.
  • Adding the dep edge without a real call site produces exactly the decorative edge ADR Verification §3 warns against; adding a half-implemented detect_ml would ship untested tensor-marshalling code in a security scanner and would break any user who supplies real weights.

So detect_ml fails 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.json provides 1000 prompts but only 100 labeled injection. 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 same use_fallback config field, but neither has a detect_ml nor emits detection_method at all, so neither can misreport. Their use_fallback field is simply unread. Left alone to keep this PR to the actual defect; worth a follow-up to remove the dead config field.

…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
nicholas-ruest merged commit cb8272e into main Jul 27, 2026
2 of 24 checks passed
@nicholas-ruest
nicholas-ruest deleted the adr/0001-real-ml-detection-and-honest-benchmarks branch July 27, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant