week_6b: Module C (The Librarian) — C.4 envelope emitter + C.0→C.4 pipeline glue#991
Open
PRAteek-singHWY wants to merge 8 commits into
Open
week_6b: Module C (The Librarian) — C.4 envelope emitter + C.0→C.4 pipeline glue#991PRAteek-singHWY wants to merge 8 commits into
PRAteek-singHWY wants to merge 8 commits into
Conversation
… once report_retrieval_recall and report_calibration each built the live pipeline (DB + embedding model + cross-encoder) independently, loading it twice and reranking every positive row twice per --use_live_embeddings run. Build it once in main and pass (retriever, reranker) into both reports, matching _build_live_pipeline's stated intent. Behavior-preserving: recall@20 285/292, rerank top-1 220/292, ECE 0.046 PASS unchanged.
…ecision gate C.3 (Week 5) produces one honest, calibrated confidence per chunk; C.4 turns it into the action — auto-link into the OpenCRE graph, or route to a human — which is the accuracy gate of the whole pipeline. - decision_engine.py: pure `decide(confidence, candidates, *, threshold, adversarial, update_ambiguous) -> DecisionResult`. Links the top-1 iff confidence >= threshold AND candidates exist AND no blocking flag; otherwise reviews with a reason_code. Reason precedence NO_CANDIDATES > ADVERSARIAL_FLAG > UPDATE_AMBIGUOUS > BELOW_THRESHOLD. Frozen result, versioned ENGINE_NAME, custom DecisionError — mirrors the C.1/C.2/C.3 model-free seams. Does not import the C.3 scaler (confidence-in -> decision-out), so it is hermetically testable. - decision_engine_test.py: 14 hermetic tests — table-driven over every confidence/flag combination, the inclusive >= boundary, all four reason codes, precedence order, and the input guards. - 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; W7 tunes tau). Informational, not a gate: the SafetyGuard flags are not wired yet, so flag-based reason codes lag until that lands. Emitters (LinkProposal/ReviewItem writers) and the C.0->C.4 pipeline glue follow in a stacked week_6b PR.
…l / ReviewItem) The decision engine (week_6) yields a verdict; this turns it into the wire contract Module D consumes. - emitter.py: `emit(section, audit, result, *, pipeline_run_id, at)` dispatches on the verdict — `linked` -> RFC LinkProposal, `review` -> RFC ReviewItem — plus the two explicit builders. Pure and timestamp-injected (no clock read) so it is hermetically testable; only builds the envelope (persistence is W8). Auto-links carry link_type "Automatically linked to" (mirrors cre_defs.LinkTypes) and the calibrated confidence; reviews carry the reason_code and a deterministic review_id derived from the chunk id. update_detection defaults to the declared degraded value (is_update=False) until the SafetyGuard lands. - emitter_test.py: 9 hermetic tests — both envelopes end-to-end, audit passthrough, degraded update_detection, no-candidates review has no suggestions, the verdict/reason guards, and link_type == cre_defs single source of truth. Stacked on week_6. Pipeline glue (C.0->C.4) follows next.
Wires the librarian end to end: section_from_queue_row (C.0) -> retriever (C.1) -> reranker (C.2) -> scaler.confidence (C.3) -> decide + emit (C.4), one envelope per valid knowledge_queue row. - pipeline.py: LibrarianPipeline.run(at=...) -> RunResult(envelopes, RunStats). Every stage is an injected seam (source/retriever/reranker/scaler), so the whole pipeline runs hermetically with stubs. Inherently dry-run — builds envelopes, never persists (queue write-back + graph writes are W8). pipeline_run_id and the timestamp are injected, never read from the clock, so a run is reproducible. Rows rejected at the C.0 boundary (e.g. UNCERTAIN) are skipped and counted, not linked. - pipeline_test.py: 5 hermetic tests — confident row auto-links, low confidence reviews (below_threshold), empty shortlist reviews (no_candidates), UNCERTAIN row skipped at the boundary, and mixed-batch counts. Stacked on the week_6b emitter.
PRAteek-singHWY
requested review from
Pa04rth,
northdpole,
paoga87 and
robvanderveer
as code owners
July 23, 2026 12:46
Contributor
Summary by CodeRabbit
WalkthroughAdds temperature-scaling calibration, deterministic decision and envelope generation, an injected librarian pipeline, live calibration and decision evaluation reports, and hermetic tests covering the new behavior. ChangesLibrarian calibration and decision pipeline
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
application/utils/librarian/pipeline.py (2)
58-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConstructor params
source/retriever/reranker/scalerare untyped.Unlike
threshold: float/pipeline_run_id: strin the same signature, and unlike the fully-typeddecision_engine.py/emitter.py, these duck-typed seams carry no type hints at all. ConsiderProtocolclasses (e.g.RetrieverLike,RerankerLike,ScalerLike) or at minimumAnyannotations formake mypyconsistency.As per coding guidelines, "Run
make mypyfor Python type checking."🤖 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/pipeline.py` around lines 58 - 73, Update the constructor in the pipeline class around __init__ to annotate source, retriever, reranker, and scaler, preferably using appropriate Protocol types such as RetrieverLike, RerankerLike, and ScalerLike; use Any only where no suitable interface exists. Preserve the existing threshold and pipeline_run_id annotations and run make mypy to verify the changes.Source: Coding guidelines
75-104: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPer-row failures in
retrieve/rerank/confidence/decide/emitabort the whole run.Only
section_from_queue_rowis guarded with try/except; a single failure from any other stage propagates and aborts the entire batch, discarding all envelopes/stats accumulated so far. This is currently safe with hermetic stubs, but the docstring states these seams are meant to become live DB/embedding/cross-encoder calls — worth hardening before that wiring lands.♻️ Suggested per-row error containment
- audit = self._retriever.retrieve(section.text) - audit = self._reranker.rerank(section.text, audit) - reranked = [c for c in audit.reranked if c.score_rerank is not None] - logits = [float(c.score_rerank) for c in reranked] - cre_ids = [c.cre_id for c in reranked] - confidence = self._scaler.confidence(logits) if logits else 0.0 - - result = decide(confidence, cre_ids, threshold=self._threshold) - envelope = emit(section, audit, result, pipeline_run_id=self._run_id, at=at) - envelopes.append(envelope) - if isinstance(envelope, LinkProposal): - linked += 1 - else: - review += 1 + try: + audit = self._retriever.retrieve(section.text) + audit = self._reranker.rerank(section.text, audit) + reranked = [c for c in audit.reranked if c.score_rerank is not None] + logits = [float(c.score_rerank) for c in reranked] + cre_ids = [c.cre_id for c in reranked] + confidence = self._scaler.confidence(logits) if logits else 0.0 + result = decide(confidence, cre_ids, threshold=self._threshold) + envelope = emit(section, audit, result, pipeline_run_id=self._run_id, at=at) + except Exception: + errored += 1 # new RunStats field + continue + envelopes.append(envelope) + if isinstance(envelope, LinkProposal): + linked += 1 + else: + review += 1🤖 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/pipeline.py` around lines 75 - 104, Update the per-item processing in run so failures from retrieve, rerank, confidence, decide, or emit are contained to that row instead of aborting the batch. Wrap the full processing pipeline after section_from_queue_row in a per-row try/except, increment the appropriate skipped/error statistic for failed rows, and continue processing later items while preserving already-created envelopes and existing successful-row counts.scripts/evaluate_librarian.py (1)
288-307: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
report_decision_accuracyre-derives the calibration set and re-fitsT, duplicatingreport_calibration's work.Both functions build the identical
positive+hard_negative(shortlist, label) set and callfit_temperatureon it (lines 234-253 inreport_calibration, lines 293-307 here). Sincemain()calls both sequentially over the sameretriever/reranker, this doubles the live per-rowreranker.rerank()(cross-encoder inference) cost with no behavioral difference — the fittedTwill be identical.Consider having
report_calibrationreturn(status, scaler)and passing the scaler intoreport_decision_accuracy, or extracting the calibration-set-building + fit into one shared helper called once frommain().🤖 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 288 - 307, The calibration data and temperature scaler are redundantly recomputed in report_decision_accuracy after report_calibration. Refactor report_calibration and main so calibration fitting occurs once, returns its status and fitted scaler, and passes that scaler into report_decision_accuracy; remove the duplicate cal_rows construction, reranker.rerank calls, label validation, and fit_temperature invocation while preserving existing skip/status behavior.
🤖 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/emitter.py`:
- Around line 52-62: Update _proposed_links so entries used as
ReviewItem.suggested_links are not labeled with AUTO_LINK_TYPE; use the
established distinct suggested-review link type if available, or omit link_type
when constructing ProposedLink. Preserve the existing confidence, rationale, and
CRE ID values.
---
Nitpick comments:
In `@application/utils/librarian/pipeline.py`:
- Around line 58-73: Update the constructor in the pipeline class around
__init__ to annotate source, retriever, reranker, and scaler, preferably using
appropriate Protocol types such as RetrieverLike, RerankerLike, and ScalerLike;
use Any only where no suitable interface exists. Preserve the existing threshold
and pipeline_run_id annotations and run make mypy to verify the changes.
- Around line 75-104: Update the per-item processing in run so failures from
retrieve, rerank, confidence, decide, or emit are contained to that row instead
of aborting the batch. Wrap the full processing pipeline after
section_from_queue_row in a per-row try/except, increment the appropriate
skipped/error statistic for failed rows, and continue processing later items
while preserving already-created envelopes and existing successful-row counts.
In `@scripts/evaluate_librarian.py`:
- Around line 288-307: The calibration data and temperature scaler are
redundantly recomputed in report_decision_accuracy after report_calibration.
Refactor report_calibration and main so calibration fitting occurs once, returns
its status and fitted scaler, and passes that scaler into
report_decision_accuracy; remove the duplicate cal_rows construction,
reranker.rerank calls, label validation, and fit_temperature invocation while
preserving existing skip/status behavior.
🪄 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: 7118b7e4-fc18-4e63-bd3f-3ab1008a3300
📒 Files selected for processing (11)
application/tests/librarian/decision_engine_test.pyapplication/tests/librarian/emitter_test.pyapplication/tests/librarian/pipeline_test.pyapplication/tests/librarian/temperature_test.pyapplication/utils/librarian/__init__.pyapplication/utils/librarian/calibration/__init__.pyapplication/utils/librarian/calibration/temperature.pyapplication/utils/librarian/decision_engine.pyapplication/utils/librarian/emitter.pyapplication/utils/librarian/pipeline.pyscripts/evaluate_librarian.py
Comment on lines
+52
to
+62
| def _proposed_links(result: DecisionResult) -> list: | ||
| """One ProposedLink per chosen CRE (top-1), carrying the calibrated confidence.""" | ||
| return [ | ||
| ProposedLink( | ||
| cre_id=cre_id, | ||
| link_type=AUTO_LINK_TYPE, | ||
| confidence=result.confidence, | ||
| rationale=None, | ||
| ) | ||
| for cre_id in result.cre_ids | ||
| ] |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the ProposedLink / suggested_links contract in the RFC schema and pydantic model.
rg -n "class ProposedLink" -A 15 application/utils/librarian/schemas.py
fd . application/utils/librarian/_rfc_schemas -e json | xargs -I{} sh -c 'echo "== {} =="; cat {}'Repository: OWASP/OpenCRE
Length of output: 9981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## emitter.py relevant sections"
sed -n '1,180p' application/utils/librarian/emitter.py
echo
echo "## schema definitions and constants"
fd -e py . application -x sh -c 'grep -n "LinkTypes|link_type|AUTO_LINK|ProposedLink|suggested_links|build_review_item|build_link_proposal" "$1" || true' sh {}Repository: OWASP/OpenCRE
Length of output: 5152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## cre_defs link type definitions"
rg -n "class LinkTypes|AutomaticallyLinkedTo|Suggested|Suggested|Proposed.*Link|link_type" -S application -g '*.py' | head -200
echo
echo "## all references to suggested_links and proposed link types"
rg -n "suggested_links|ProposedLink\\(|link_type\\s*=|LinkTypes\\.Autom|Auto.*Link" -S application tests 2>/dev/null || true
echo
echo "## tests around emitted review/link proposal"
fd -e py . application tests -x sh -c 'grep -n "build_review_item\\|build_link_proposal\\|LinkProposal|ReviewItem|suggested_links\\[" "$1" || true' sh {}Repository: OWASP/OpenCRE
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## focused cre_defs.py/link type refs"
fd -e py cre_defs application cre_ -x sh -c 'grep -n -C 4 "AutomaticallyLinkedTo\\|LinkTypes\\|Automatically.*Link\\|Suggested" "$1" || true' sh {}
echo
echo "## focused tests for emitted envelope and suggested link type"
fd -e py . tests test_ -x sh -c 'grep -n -C 5 "suggested_links\\|build_review_item\\|build_link_proposal\\|link_type\\|Automatically linked to" "$1" || true' sh {}Repository: OWASP/OpenCRE
Length of output: 2218
Avoid labeling unlinked review suggestions as auto-links.
_proposed_links always emits link_type="Automatically linked to", but build_review_item uses it for ReviewItem.suggested_links, which are candidate links pending human review. Add a distinct suggested-review link type or omit link_type for these entries; otherwise consumers see an established auto-link where only a suggestion exists.
🤖 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/emitter.py` around lines 52 - 62, Update
_proposed_links so entries used as ReviewItem.suggested_links are not labeled
with AUTO_LINK_TYPE; use the established distinct suggested-review link type if
available, or omit link_type when constructing ProposedLink. Preserve the
existing confidence, rationale, and CRE ID values.
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
1 participant
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.
Hi @northdpole — Week 6b of Module C, stacked on the Week-6 decision engine. Week 6 produced the verdict (auto-link vs. review); this PR turns that verdict into the wire envelopes Module D consumes, and wires the whole C.0→C.4 pipeline end to end (dry-run).
Overview
Week 6 gave us
decide()→ aDecisionResult. Two things were deliberately left out of that PR to keep it a clean, provable unit: emitting the RFC envelope, and wiring the stages together. This PR adds both.This PR's role:
The emitter (
emitter.py) —emit(section, audit, result, *, pipeline_run_id, at)dispatches on the verdict:linked→ an RFCLinkProposal,review→ an RFCReviewItem. It only builds the envelope (persisting it is W8). Pure and timestamp-injected (no clock read), so every branch is hermetically testable. Auto-links carrylink_type"Automatically linked to" (mirrorscre_defs.LinkTypes) and the calibrated confidence; reviews carry thereason_codeand a deterministicreview_idderived from the chunk id.update_detectiondefaults to the declared-degraded value (is_update=False) until the SafetyGuard lands.The pipeline (
pipeline.py) —LibrarianPipeline.run(at=...)runs C.0→C.4 over a knowledge source:section_from_queue_row(C.0) →retrieve(C.1) →rerank(C.2) →scaler.confidence(C.3) →decide+emit(C.4), one envelope per valid row. Every stage is an injected seam, so the whole pipeline runs hermetically with stubs. Inherently dry-run — builds envelopes, never persists.pipeline_run_idand the timestamp are injected, never read from the clock, so a run is reproducible.Scope: 2 new modules + 2 new tests. No frontend, no migration, no behaviour change to OpenCRE proper.
What changed
emitter.py(new)emit()+build_link_proposal/build_review_item.DecisionResult→ RFCLinkProposal/ReviewItem, snapshotting the chunk (KnowledgeSnapshot) and passing the C.1/C.2RetrievalAuditthrough untouched. Pure, timestamp-injected, customEmitterError; auto-linklink_typemirrorscre_defs.LinkTypes.AutomaticallyLinkedTo; degradedupdate_detectiondefault; deterministicreview_id.pipeline.py(new)LibrarianPipeline.run(at=...) -> RunResult(envelopes, RunStats). Wires all five stages via injected seams (source/retriever/reranker/scaler), inherently dry-run. Rows rejected at the C.0 boundary (e.g.UNCERTAIN) are skipped and counted, not linked.emitter_test.py(new),pipeline_test.py(new)update_detection, no-candidates review has no suggestions, the verdict/reason guards, andlink_type == cre_defssingle source of truth. Pipeline (5): confident row auto-links, low confidence reviews (below_threshold), empty shortlist reviews (no_candidates),UNCERTAINrow skipped at the boundary, and mixed-batch counts.How the pieces connect
flowchart TB row["knowledge_queue row"] subgraph PIPE["LibrarianPipeline.run (this PR)"] c0["C.0 section_from_queue_row"] c1["C.1 retriever.retrieve"] c2["C.2 reranker.rerank"] c3["C.3 scaler.confidence"] c4["C.4 decide()"] emit["emit()"] c0 --> c1 --> c2 --> c3 --> c4 --> emit end row --> c0 emit --> lp["LinkProposal (linked)"] emit --> ri["ReviewItem (review + reason_code)"] skip["UNCERTAIN / invalid row -> skipped, counted"] c0 -.-> skipResults
The emitter and pipeline are pure and dry-run — every branch is covered by the hermetic tests above, no live key required. The end-to-end demo on the golden set (populated envelopes for a full slice) is the midterm deliverable; this PR lands the machinery it runs on.
What is intentionally not here
ood/conformal/update_detector) that would populate theadversarial/update_ambiguousflags and realupdate_detection.cre_maindispatch and persistence / queue write-back / graph writes (W8) — the pipeline stays dry-run; nothing is written to OpenCRE.KnowledgeQueueItemmirror over the golden fixture, not a live connection to Module B.How to verify locally