week_4: address CodeRabbit findings on #957

- run_librarian: fix stale docstring (C.2 rerank is built), guard the
  semantic retrieve/rerank per-section so one bad section can't abort the
  dry-run batch
- knowledge_source: log ValidationError.errors(include_input=False), never
  the raw queue row (no content leak)
- build_golden_dataset: fail loud on ambiguous ASVS->CRE mappings instead of
  silently picking one (matches build_explicit/build_update)
- candidate_retriever: stable descending sort so tied cosine scores are
  deterministic
- cross_encoder: zip(..., strict=True) (B905)
- config_loader_test: annotate OVERRIDES as ClassVar (RUF012)
- schemas: centralize the schema_version check across the three envelopes
- section_validator: extract _validate_or_raise helper for both call sites
- requirements: pin sentence-transformers>=5.0,<6.0 (tested with 5.6)
- evaluate_librarian: drop the no-op --dry_run flag (harness never writes)

Author: PRAteek-singHWY

application/cmd/cre_main.py

@@ -1073,10 +1073,10 @@ def run_librarian(
 
     For each knowledge-queue section: try the deterministic explicit-CRE fast
     path (C.0.5); on no/ambiguous reference, run the semantic retriever (C.1)
-    and log the top-K candidate shortlist. The cross-encoder rerank (C.2, W4),
-    decision/threshold routing (C.3-C.4, W5) and graph writes (W8) are not
-    built yet, so this is dry-run only: it never writes a link. ``--run_librarian``
-    without writes behaves identically and warns.
+    and rerank its top-K shortlist with the cross-encoder (C.2, W4), then log
+    the reranked candidates. Decision/threshold routing (C.3-C.4, W5) and graph
+    writes (W8) are not built yet, so this is dry-run only: it never writes a
+    link. ``--run_librarian`` without writes behaves identically and warns.
 
     Ops note: this is opt-in CLI only — it is not on the ``Procfile`` and is
     wired into neither the web app nor the background worker (that lands W8).
@@ -1172,9 +1172,19 @@ def run_librarian(
             logger.info("[explicit] %s -> %s", section.chunk_id, resolution.cre_ids[0])
             continue
 
+        try:
+            audit = retriever.retrieve(section.text)
+            audit = reranker.rerank(section.text, audit)
+        except Exception as exc:  # noqa: BLE001 - one bad section must not abort the batch
+            rejected += 1
+            logger.warning(
+                "[semantic] %s skipped: retrieval/rerank failed: %s",
+                section.chunk_id,
+                exc,
+            )
+            continue
+
         semantic += 1
-        audit = retriever.retrieve(section.text)
-        audit = reranker.rerank(section.text, audit)
         top = ", ".join(
             f"{c.cre_id}:{c.score_rerank:.3f}" for c in audit.reranked
         )

application/tests/librarian/config_loader_test.py

@@ -1,6 +1,7 @@
 import os
 import unittest
 from dataclasses import FrozenInstanceError
+from typing import ClassVar, Dict
 from unittest import mock
 
 from application.utils.librarian.config_loader import LibrarianConfig, load_config
@@ -27,7 +28,7 @@ def test_config_is_frozen(self):
 
 
 class TestConfigLoaderOverrides(unittest.TestCase):
-    OVERRIDES = {
+    OVERRIDES: ClassVar[Dict[str, str]] = {
         "CRE_LIBRARIAN_RETRIEVER_BACKEND": "pgvector",
         "CRE_LIBRARIAN_CROSSENCODER_MODEL": "cross-encoder/other",
         "CRE_LIBRARIAN_TOP_K_RETRIEVAL": "50",

application/utils/librarian/candidate_retriever.py

@@ -157,7 +157,9 @@ def retrieve(self, text: str) -> RetrievalAudit:
         # Top-K by descending score. argsort is ascending, so take the tail
         # and reverse; cap at pool size when the hub is smaller than K.
         k = min(self._top_k, len(self._pool.cre_ids))
-        top_idx = np.argsort(scores)[-k:][::-1]
+        # Stable descending sort so tied cosine scores keep hub (index) order —
+        # deterministic across runs. argsort(-scores) descends; kind="stable".
+        top_idx = np.argsort(-scores, kind="stable")[:k]
 
         candidates: List[CreCandidate] = [
             CreCandidate(

application/utils/librarian/cross_encoder.py

@@ -100,7 +100,7 @@ def rerank(self, text: str, audit: RetrievalAudit) -> RetrievalAudit:
 
         reranked = [
             c.model_copy(update={"score_rerank": float(s)})
-            for c, s in zip(candidates, scores)
+            for c, s in zip(candidates, scores, strict=True)
         ]
         # Highest cross-encoder score first, then keep only the best top_n.
         # Python's sort is stable, so ties preserve C.1's cosine order.

application/utils/librarian/knowledge_source.py

@@ -38,9 +38,11 @@ def items(self) -> Iterator[KnowledgeQueueItem]:
                     try:
                         yield KnowledgeQueueItem.model_validate_json(line)
                     except ValidationError as exc:
+                        # Log only error locations/types, never the raw input —
+                        # queue rows can carry sensitive content.
                         logger.warning(
                             "Skipping malformed knowledge_queue row at line %d: %s",
                             line_no,
-                            exc,
+                            exc.errors(include_input=False),
                         )
                         continue

application/utils/librarian/schemas.py

@@ -22,6 +22,15 @@
 _SCHEMA_VERSION_RE = re.compile(r"^0\.\d+\.\d+$")
 
 
+def _require_schema_version(value: str) -> None:
+    """Shared envelope check — every RFC envelope pins schema_version to 0.x.y.
+
+    Centralized so the regex and message can't drift across envelopes.
+    """
+    if not _SCHEMA_VERSION_RE.match(value):
+        raise ValueError(r"schema_version must match ^0\.\d+\.\d+$")
+
+
 # ---------- Enums (RFC) ----------
 
 
@@ -212,8 +221,7 @@ class KnowledgeItem(BaseModel):
 
     @model_validator(mode="after")
     def _rfc_rules(self) -> "KnowledgeItem":
-        if not _SCHEMA_VERSION_RE.match(self.schema_version):
-            raise ValueError(r"schema_version must match ^0\.\d+\.\d+$")
+        _require_schema_version(self.schema_version)
         if self.status == KnowledgeStatus.accepted and self.content is None:
             raise ValueError("status=accepted requires content")
         if self.status == KnowledgeStatus.rejected and self.rejection is None:
@@ -238,8 +246,7 @@ class LinkProposal(BaseModel):
 
     @model_validator(mode="after")
     def _schema_version_pattern(self) -> "LinkProposal":
-        if not _SCHEMA_VERSION_RE.match(self.schema_version):
-            raise ValueError(r"schema_version must match ^0\.\d+\.\d+$")
+        _require_schema_version(self.schema_version)
         return self
 
 
@@ -262,8 +269,7 @@ class ReviewItem(BaseModel):
 
     @model_validator(mode="after")
     def _schema_version_pattern(self) -> "ReviewItem":
-        if not _SCHEMA_VERSION_RE.match(self.schema_version):
-            raise ValueError(r"schema_version must match ^0\.\d+\.\d+$")
+        _require_schema_version(self.schema_version)
         return self
 
 

application/utils/librarian/section_validator.py

@@ -32,9 +32,9 @@
 """
 
 from dataclasses import dataclass
-from typing import Any, Dict, Optional, Union
+from typing import Any, Dict, Optional, Type, TypeVar, Union
 
-from pydantic import ValidationError
+from pydantic import BaseModel, ValidationError
 
 from application.utils.librarian.schemas import (
     KnowledgeItem,
@@ -46,6 +46,8 @@
     SourceType,
 )
 
+_ModelT = TypeVar("_ModelT", bound=BaseModel)
+
 KNOWLEDGE_LABEL = "KNOWLEDGE"
 
 # MVP scope: golden dataset and CRE hub vectors are English-only.
@@ -86,6 +88,17 @@ class Section:
     locator: Locator
 
 
+def _validate_or_raise(model_cls: Type[_ModelT], raw: Any) -> _ModelT:
+    """Coerce a raw row into ``model_cls``, converting Pydantic's ValidationError
+    into a typed MalformedKnowledgeItemError so it never escapes this module."""
+    if isinstance(raw, model_cls):
+        return raw
+    try:
+        return model_cls.model_validate(raw)
+    except ValidationError as exc:
+        raise MalformedKnowledgeItemError(str(exc)) from exc
+
+
 def _require_text(text: str) -> str:
     if not text or not text.strip():
         raise EmptyTextError("section text is empty or whitespace-only")
@@ -111,11 +124,7 @@ def section_from_queue_row(
 
     Raises a SectionValidationError subclass on any rejection.
     """
-    if not isinstance(row, KnowledgeQueueItem):
-        try:
-            row = KnowledgeQueueItem.model_validate(row)
-        except ValidationError as exc:
-            raise MalformedKnowledgeItemError(str(exc)) from exc
+    row = _validate_or_raise(KnowledgeQueueItem, row)
 
     if row.llm_label != KNOWLEDGE_LABEL:
         raise NotKnowledgeError(
@@ -154,11 +163,7 @@ def section_from_knowledge_item(
 
     Raises a SectionValidationError subclass on any rejection.
     """
-    if not isinstance(item, KnowledgeItem):
-        try:
-            item = KnowledgeItem.model_validate(item)
-        except ValidationError as exc:
-            raise MalformedKnowledgeItemError(str(exc)) from exc
+    item = _validate_or_raise(KnowledgeItem, item)
 
     if item.status != KnowledgeStatus.accepted:
         raise NotKnowledgeError(

requirements.txt

@@ -96,7 +96,7 @@ redis
 ruamel.yaml
 ruamel.yaml.clib
 scikit-learn
-sentence-transformers
+sentence-transformers>=5.0.0,<6.0.0
 Shapely
 six
 smmap

scripts/build_golden_dataset.py

@@ -195,19 +195,27 @@
 
 
 def _fetch_asvs_cre(conn: sqlite3.Connection, section_id: str) -> Optional[str]:
-    row = conn.execute(
+    rows = conn.execute(
         """
-        SELECT c.external_id
+        SELECT DISTINCT c.external_id
         FROM node n
         JOIN cre_node_links l ON l.node = n.id
         JOIN cre c ON c.id = l.cre
         WHERE n.name LIKE '%ASVS%' AND n.section_id = ?
         ORDER BY c.external_id
-        LIMIT 1
         """,
         (section_id,),
-    ).fetchone()
-    return row[0] if row else None
+    ).fetchall()
+    if not rows:
+        return None
+    if len(rows) > 1:
+        # Fail loud rather than silently pick one — an ambiguous section would
+        # bake a wrong CRE into the golden set (matches build_explicit/build_update).
+        raise ValueError(
+            f"ASVS section {section_id!r} maps to multiple CREs "
+            f"({', '.join(r[0] for r in rows)}); cannot pick a golden CRE"
+        )
+    return rows[0][0]
 
 
 def build_positive_asvs(conn: sqlite3.Connection) -> List[Dict]:

scripts/evaluate_librarian.py

@@ -203,9 +203,6 @@ def main(argv: List[str]) -> int:
     parser.add_argument("--threshold", type=float, default=cfg.link_threshold)
     parser.add_argument("--top_k_retrieval", type=int, default=cfg.top_k_retrieval)
     parser.add_argument("--top_k_rerank", type=int, default=cfg.top_k_rerank)
-    parser.add_argument(
-        "--dry_run", action="store_true", help="no writes (always true pre-W8)"
-    )
     parser.add_argument(
         "--no_hub_firewall",
         action="store_true",