week_6: Module C (The Librarian) — C.4 decision engine + golden-set decision gate

[PRAteek-singHWY]

Hi @northdpole - Week 6 of Module C. Week 5 produced one honest confidence per chunk; this PR turns that number into the actual decision - auto-link into the graph, or route to a human - which is the accuracy gate of the whole pipeline.

Stacked on #974 (Week 5). Based on gsocmodule_C_week_5; only the top commit is new. Until #974 merges the diff shows the W5 commits too - I'll rebase onto main as it lands, shrinking it to the Week-6-only surface (3 files). No dependency on Modules A or B: this runs entirely on the golden dataset.

Overview

Week 3 built the search step (C.1), Week 4 the rerank step (C.2), and Week 5 the calibration step (C.3) - a single scalar T that maps the reranked shortlist to a trustworthy confidence = softmax(logits / T).

The problem: a calibrated confidence is only useful if something acts on it. Auto-linking a wrong CRE pollutes the graph; sending everything to a human defeats the point. We need a rule that auto-links when it's safe and escalates when it isn't.

This PR's role: build the decision step (C.4) - decision_engine.decide(). It links the top-1 candidate iff confidence >= threshold and there is a candidate and no blocking safety flag; otherwise it routes to review with a reason_code. It's a pure function of (confidence, candidates, flags, threshold) -> DecisionResult - it does not import the C.3 scaler (confidence-in → decision-out), so it stays model-free and hermetically testable, mirroring the C.1/C.2/C.3 seams. Reason-code precedence is total: NO_CANDIDATES > ADVERSARIAL_FLAG > UPDATE_AMBIGUOUS > BELOW_THRESHOLD.

The harness gains a decision-accuracy gate: run the live C.1→C.4 decision over the golden set and measure how often decide() lands on the expected auto-link-vs-review call.

Scope: 1 new module + 1 new test + additive harness wiring. The SafetyGuard flags (adversarial / update_ambiguous) are accepted by decide() but not yet produced - nothing sets them, so they default to False (declared-degraded until that lands). No frontend, no migration, no behaviour change to OpenCRE proper.

What changed

Area	Files	Description
C.4 decision engine	decision_engine.py (new)	decide(confidence, candidate_cre_ids, *, threshold, adversarial, update_ambiguous) -> DecisionResult. Links the top-1 iff confidence >= threshold AND candidates exist AND no blocking flag; else reviews with a reason_code. Frozen DecisionResult, versioned ENGINE_NAME, custom DecisionError, input guards on confidence/threshold. Model-free and confidence-agnostic so it is hermetically testable - mirrors the C.1 embed_fn / C.2 score_fn / C.3 scaler seams.
Eval harness	evaluate_librarian.py	additive report_decision_accuracy: fits T on positive+hard_negative, runs the live C.1→C.4 decision over the golden set, and reports overall agreement plus auto-link recall vs review recall (a single accuracy hides that at tau=0.80 the softmax top-1 mass of a correct-but-close winner is often ~0.5, so correct positives route to review - the safe direction). Informational, not a hard gate: tuning tau is the Week-7 experiment, and flag-based reason codes lag until the SafetyGuard lands. report_calibration (W5) untouched.
Tests	decision_engine_test.py (new)	14 hermetic tests - table-driven over every confidence/flag combination, the inclusive >= boundary, all four reason codes, the precedence order, and the input guards.

How the pieces connect

flowchart TB
    conf["C.3 calibrated confidence<br/>+ reranked candidates + flags"]
    subgraph C4["C.4 - decision engine (this PR)"]
        rule["decide(): confidence at or above tau ?<br/>AND candidates exist AND no blocking flag"]
        res["DecisionResult<br/>(decision, confidence, cre_ids, reason_code)"]
        rule --> res
    end
    conf --> rule
    res --> link["linked -> LinkProposal (W6b emits)"]
    res --> review["review -> ReviewItem + reason_code<br/>NO_CANDIDATES / ADVERSARIAL_FLAG /<br/>UPDATE_AMBIGUOUS / BELOW_THRESHOLD"]

Loading

Results

# offline (CI default) - hermetic, no key/DB/model
127 librarian tests passing (113 from W1–W5 + 14 new; 1 skipped)
explicit slice (C.0.5 resolver): 5/5 - gate 100%: PASS

# live (local, against the migrated embedding_vec cache: 428 CRE hub vectors,
# gemini/gemini-embedding-001 dim 3072, ms-marco-MiniLM-L-6-v2, hub-firewall ON)
retrieval recall@20 (C.1): any-hit 285/292 (98%)
rerank top-1     (C.2): 220/292 (75%)
calibration (C.3, 304 rows): T=1.105; ECE 0.046 (calibrated); gate ECE<0.10: PASS
decision (C.4, 319 rows @ tau=0.80): overall 181/319 (57%)
  auto-link recall (expected-linked): 176/314 (56%)
  review recall   (expected-review): 5/5 (100%); reason_code 4/5 (80%)

Reading the C.4 line by direction, because a single accuracy hides the story:

• Review recall 5/5 (100%) - every chunk that should go to a human does. The engine never wrongly auto-links something that needs review. For an accuracy gate, this is the number that matters most.
• Auto-link recall 56% - at tau=0.80 many correct-but-close positives fall below the bar and route to review (the safe direction). This is a conservative starting point; Week 7's threshold sweep is exactly the lever that lifts it.
• reason_code 4/5 - the one miss is a flag-based code that needs the SafetyGuard (not wired; declared).

What is intentionally not here

• Envelope emitters + pipeline glue (W6b) - turning a DecisionResult into the RFC LinkProposal / ReviewItem and wiring C.0→C.4. Ships stacked as week_6b.
• SafetyGuard flags (ood / conformal / update-detection) that would populate adversarial / update_ambiguous.
• Persisting the fitted T for the live decision path, and live B→C integration + graph writes (W8) - the pipeline stays dry-run.

How to verify locally

# the new decision-engine tests (hermetic - no key, DB, or model)
python3 -m unittest application.tests.librarian.decision_engine_test
# or the whole librarian suite
python3 -m unittest discover -s application/tests/librarian -p '*_test.py' -t .

# live decision gate (needs a populated cache DB + an embedding-capable LLM).
# after the pgvector migration (#979), migrate a legacy SQLite cache first:
python3 scripts/rewrite_sqlite_embeddings_to_vec.py --db standards_cache.sqlite
python3 scripts/evaluate_librarian.py \
    --dataset application/tests/librarian/fixtures/golden_dataset.json \
    --use_live_embeddings --cache_file standards_cache.sqlite

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Summary by CodeRabbit

• New Features

  • Added confidence calibration for more reliable match probabilities.
  • Added automatic routing for high-confidence links and human review of uncertain or flagged matches.
  • Added live evaluation reports for calibration quality and decision accuracy.
  • Live evaluations now reuse retrieval and ranking results for improved efficiency.

• Tests

  • Added comprehensive coverage for calibration, decision thresholds, safety flags, evaluation behavior, and duplicate dataset IDs.

• Documentation

  • Updated librarian documentation to describe calibration and decision routing.

Walkthrough

Adds temperature-scaling calibration, confidence-based link/review decisions, hermetic tests, and live evaluation reporting with shared retrieval and reranking audits.

Changes

Librarian calibration and routing

Layer / File(s)	Summary
Temperature calibration API and validation application/utils/librarian/calibration/..., application/utils/librarian/__init__.py, application/tests/librarian/temperature_test.py	Adds temperature scaling, NLL fitting, ECE calculation, validation errors, version metadata, calibration documentation, and hermetic tests.
Decision routing contract and tests application/utils/librarian/decision_engine.py, application/tests/librarian/decision_engine_test.py	Adds immutable decision results, input guards, precedence rules, and tests for link/review outcomes and reason codes.
Live evaluation orchestration and tests scripts/evaluate_librarian.py, application/tests/librarian/evaluate_harness_test.py, application/tests/librarian/dataset_test.py	Builds the live pipeline once, reuses shared audits across reports, evaluates calibration and decision accuracy, rejects duplicate dataset IDs, and returns calibration status from main(). Tests verify audit reuse and report behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• OWASP/OpenCRE#922: Provides the librarian schemas, golden dataset, and harness used by this change.
• OWASP/OpenCRE#957: Adds related librarian evaluation-pipeline behavior consumed by this change.
• OWASP/OpenCRE#974: Adds related calibration and shared evaluation-harness changes.

Suggested reviewers: pa04rth, paoga87, robvanderveer

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the Week 6 decision engine and golden-set decision gate, which are the primary changes.
Description check	✅ Passed	The description directly explains the decision engine, evaluation harness changes, tests, results, and deferred scope.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

application/utils/librarian/calibration/temperature.py (1)

63-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication between _softmax_top and probabilities.

Both implement the same "asarray + empty check + softmax" logic independently. confidence() could derive from probabilities() instead of a separate module-level helper, keeping the empty-shortlist guard in one place.

♻️ Suggested consolidation

     def confidence(self, logits: Sequence[float]) -> float:
         """P(the top candidate is correct) — the top-1 mass of the softmax.

         This is the number the W6 decision engine thresholds on.
         """
-        return _softmax_top(logits, self.temperature)
+        return float(self.probabilities(logits).max())

Also applies to: 105-117

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@application/utils/librarian/calibration/temperature.py` around lines 63 - 68,
Consolidate the duplicated shortlist conversion, empty-check, and softmax logic
by removing or bypassing `_softmax_top` and deriving `confidence()` from the
existing `probabilities()` implementation. Ensure `probabilities()` remains the
single guard for empty candidate shortlists while preserving the top-1
probability result and temperature behavior.

scripts/evaluate_librarian.py (1)

234-244: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

Live retrieve+rerank (and the temperature fit) is redundantly recomputed 2-3x per row.

report_decision_accuracy rebuilds the exact same cal_rows set and reruns retriever.retrieve/reranker.rerank per row to refit a second TemperatureScaler, duplicating work report_calibration (called right before it in main, L458-459) already did. Then its own graded loop reruns retrieve/rerank again for rows that overlap with cal_rows (e.g. positive-slice rows with an expected decision). Since retrieval/reranking against a live embedding model + cross-encoder is the expensive part this harness gates behind --use_live_embeddings, this triples model calls for no functional benefit — the fit and audits are deterministic given the same inputs.

Consider having report_calibration return the fitted TemperatureScaler (and/or the per-row audits) so report_decision_accuracy reuses them instead of recomputing, and caching each row's retrieve+rerank result by row id so the cal_rows/graded loops don't redo live calls for the same row.

Also applies to: 292-333

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/evaluate_librarian.py` around lines 234 - 244, Refactor
report_calibration and report_decision_accuracy to reuse the fitted
TemperatureScaler and per-row rerank audits instead of rerunning retrieve and
rerank. Have report_calibration return the scaler and/or cached audits, pass
them from main into report_decision_accuracy, and ensure overlapping cal_rows
and graded rows retrieve each row only once, keyed by a stable row identifier.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@application/utils/librarian/calibration/__init__.py`:
- Around line 1-13: Update the package docstring in the module-level
documentation to describe the implemented shortlist-wide softmax calibration
used by temperature.py, replacing the single-logit sigmoid formula and related
claims. Explain that logits are scaled by a fitted scalar temperature and
normalized across each candidate shortlist, while preserving the existing
purpose, NLL fitting, and ECE context.

---

Nitpick comments:
In `@application/utils/librarian/calibration/temperature.py`:
- Around line 63-68: Consolidate the duplicated shortlist conversion,
empty-check, and softmax logic by removing or bypassing `_softmax_top` and
deriving `confidence()` from the existing `probabilities()` implementation.
Ensure `probabilities()` remains the single guard for empty candidate shortlists
while preserving the top-1 probability result and temperature behavior.

In `@scripts/evaluate_librarian.py`:
- Around line 234-244: Refactor report_calibration and report_decision_accuracy
to reuse the fitted TemperatureScaler and per-row rerank audits instead of
rerunning retrieve and rerank. Have report_calibration return the scaler and/or
cached audits, pass them from main into report_decision_accuracy, and ensure
overlapping cal_rows and graded rows retrieve each row only once, keyed by a
stable row identifier.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3c77096d-5a49-4d8f-9eee-e6a67592cb12

📥 Commits

Reviewing files that changed from the base of the PR and between a55e380 and fccfaab.

📒 Files selected for processing (7)

• application/tests/librarian/decision_engine_test.py
• application/tests/librarian/temperature_test.py
• application/utils/librarian/__init__.py
• application/utils/librarian/calibration/__init__.py
• application/utils/librarian/calibration/temperature.py
• application/utils/librarian/decision_engine.py
• scripts/evaluate_librarian.py

[northdpole]

Maintainer review — Module C Week 6 (#990)

Decision engine looks solid: precedence, inclusive τ, guards, and table tests are clean. No blocking logic bugs in the unique Week-6 surface.

Depends on #974 for the stack base (please rebase after that gate fix lands). One docstring nit inline.

Non-blocking note: report_decision_accuracy intentionally always returns 0 (informational until SafetyGuard + τ tuning) — fine; just don't confuse that with the ECE gate in #974.

[northdpole]

Nit — package docstring is stale

This still says "Decision routing (C.4, W6) onward is not built yet." Week 6 adds decide() here. Please update the scope blurb to mention C.4 / W6 (and leave W6b emitter/pipeline / W8 writers as not-yet if you prefer).

[PRAteek-singHWY]

Fixed in 86d3d5b5. Added the W6 (C.4) scope line for decide() and moved the not-yet marker down to the W6b emitter/pipeline glue and the W8 queue/graph writers. Will rebase onto gsocmodule_C_week_5 once #974 lands.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

application/tests/librarian/evaluate_harness_test.py (1)

211-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise the decision-report behavior that these tests name.

test_grades_expected_decision_rows_off_shared_audits passes if report_decision_accuracy() returns 0 without reporting any metrics. Assert the reported counts or return structured metrics.

test_no_graded_rows_is_not_an_error creates an audit for a row that _golden_row() marks as linked. The graded list is therefore nonempty. Pass an empty audit map to execute the no-graded-rows branch.

As per coding guidelines, “New behavior and importers should follow a test-first workflow.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@application/tests/librarian/evaluate_harness_test.py` around lines 211 - 247,
Strengthen the tests around harness.report_decision_accuracy by asserting its
reported decision metrics or structured result, rather than only its status and
pipeline-call counts. In test_no_graded_rows_is_not_an_error, pass an empty
audit map so the no-graded-rows branch is exercised instead of creating a linked
audit from _golden_row; preserve the expected zero status.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@application/tests/librarian/evaluate_harness_test.py`:
- Line 123: Update the assignment from harness.report_calibration in the test to
bind the unused status result to _, while preserving the scaler binding and
existing test behavior.

In `@scripts/evaluate_librarian.py`:
- Around line 110-113: Update the live-evaluation descriptions in
_build_live_pipeline(), the --use_live_embeddings help text, and the offline
message to state that the live pipeline produces the C.4 decision report and may
return a nonzero calibration status.

---

Nitpick comments:
In `@application/tests/librarian/evaluate_harness_test.py`:
- Around line 211-247: Strengthen the tests around
harness.report_decision_accuracy by asserting its reported decision metrics or
structured result, rather than only its status and pipeline-call counts. In
test_no_graded_rows_is_not_an_error, pass an empty audit map so the
no-graded-rows branch is exercised instead of creating a linked audit from
_golden_row; preserve the expected zero status.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: e0d5434e-4076-49a9-b00c-d711340a56dc

📥 Commits

Reviewing files that changed from the base of the PR and between 86d3d5b and 6ef7865.

📒 Files selected for processing (3)

• application/tests/librarian/evaluate_harness_test.py
• application/utils/librarian/calibration/temperature.py
• scripts/evaluate_librarian.py

🚧 Files skipped from review as they are similar to previous changes (1)

• application/utils/librarian/calibration/temperature.py

[PRAteek-singHWY]

Pushed on top of the docstring fix.

Carried the #974 fixes onto this branch. This branch is cumulative off main, so it already contained the W5 calibration code including the old return 0. All three PRs now show the same corrected code, and the duplicate commits will drop by patch-id when this rebases onto #974 as you asked.

report_decision_accuracy no longer re-derives or re-fits. It rebuilt the positive + hard_negative calibration set from its own retrieve and rerank pass and fit its own temperature, duplicating what report_calibration had done a few lines earlier. On a live run that meant a third pipeline pass over the calibration slices and two independent fits of the same T, with nothing guaranteeing the two agreed. Now:

• calibration_set() extracts the (shortlist, label) derivation, so the C.3 gate and the C.4 report read the same pairs off the same shared audits.
• report_calibration returns (status, scaler); report_decision_accuracy takes the fitted scaler, so C.4 thresholds on exactly the T the ECE gate measured.
• A degenerate set yields (1, None) and C.4 is skipped with a message: no fitted T means no honest confidence to threshold, and the run has already failed.
• The live audit set now covers expected-decision rows too. C.4 grades those and they are not confined to the positive/hard_negative slices, so keying them off the calibration slices alone would have dropped them.

Adds evaluate_harness_test.py. The live reports only run under --use_live_embeddings, so nothing exercised their wiring, which is exactly the code that has to share one pipeline pass and one T across three reports. A counting stub asserts the pipeline is called once per row and not once per report, that only the two calibration slices enter the fit, and that a degenerate set returns status 1 with no scaler rather than reporting success. Verified that last one fails if the gate is flipped back to 0.

134 librarian tests pass; the hermetic harness run still exits 0.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@application/tests/librarian/dataset_test.py`:
- Around line 113-120: Update _load_harness to validate that spec from
spec_from_file_location is not None before passing it to module_from_spec or
accessing its loader. If the specification cannot be created, raise a clear
error indicating that scripts/evaluate_librarian.py is missing.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: e37bcc6d-95de-4455-a3ae-cf975c7b6f14

📥 Commits

Reviewing files that changed from the base of the PR and between 6ef7865 and 90992f5.

📒 Files selected for processing (3)

• application/tests/librarian/dataset_test.py
• application/tests/librarian/evaluate_harness_test.py
• scripts/evaluate_librarian.py

🚧 Files skipped from review as they are similar to previous changes (1)

• application/tests/librarian/evaluate_harness_test.py

[northdpole]

Maintainer review — Module C Week 6 (#990) — refresh

Verdict: Logic/tests look good; not mergeable yet until black is green and the Week-5 base is settled.

What looks solid

• decide() is the right size: pure, model-free, inclusive >= τ, top-1 only, frozen DecisionResult
• Reason-code precedence matches the RFC story: NO_CANDIDATES > ADVERSARIAL_FLAG > UPDATE_AMBIGUOUS > BELOW_THRESHOLD
• Hermetic table tests cover boundary, flags, guards, and precedence
• Declared-degraded SafetyGuard flags (accepted, not produced) is clearly called out — fine for W6
• Decision-accuracy report staying informational (always status 0) is the right call until W7 τ sweep / SafetyGuard

Blockers

1. Lint / black — CI fails on application/tests/librarian/evaluate_harness_test.py (would reformat). Please run black and push.
2. Stack — still depends on #974 (CHANGES_REQUESTED). Rebase onto main after that lands so the Week-6-only surface is what we merge.

Non-blocking

• Confirm harness tests assert the graded decision outcomes they name (CodeRabbit called this out earlier) — nice-to-have, not a gate for W6.

Once black is green and #974 is in (or this is rebased cleanly), this is approve-ready.

[northdpole]

#974 is merged. Please rebase this PR onto main, run black (CI was failing on evaluate_harness_test.py), and push — happy to re-approve once green.

[northdpole]

Rebase needed (blocking): this PR is CONFLICTING with main after #974 merged (~9 commits behind).

Please:

1. Rebase onto latest main
2. Run black on application/tests/librarian/evaluate_harness_test.py (lint still red)
3. Push and ping for re-review

Stack order remains #990 → #991.

[northdpole]

Re-review (post-rebase)

Prior blocker cleared: MERGEABLE on main, black/lint clean, all 6 checks green. Thanks for the Slack ping and the stack cleanup.

Verified

• decide() is a small total function with clear reason-code precedence; hermetic table tests look solid.
• Harness shares one retrieve+rerank pass and one fitted T across C.3/C.4 reports; C.4 stays informational; degenerate calibration still fails the live run (no greenwash).
• CodeRabbit harness-load note addressed in dataset_test._load_harness: ImportError with path, no skip — correct call.
• Package / calibration docstrings refreshed for W6 scope.

Optional follow-up (non-blocking)

evaluate_harness_test.py still uses a bare assert _spec and _spec.loader at import time; aligning it with the same ImportError(f"... {path}") pattern as dataset_test would be nice consistency, not a merge blocker.

Approve. Merge with rebase after CI; then land #991 (stacked on this).