From b3ce0d9406534b78fad11a60174ea5fd15cff2ff Mon Sep 17 00:00:00 2001 From: Ramiro Cantu Date: Sun, 31 May 2026 22:04:53 -0500 Subject: [PATCH 1/2] feat(kb): document-level learning-goals fact extraction (RCA-11) Rework the PDF atomic-fact extractor (pdf-vision-v1 -> v2) to stop over-extracting lecture-context noise. - Vision transcription enriched to describe diagrams/figures/graphs so visual facts survive the transcription bottleneck. - Extraction is now document-level + learning-goals-driven synthesis with a concept/context discernment gate; context facts are dropped at persist and counted (IngestReport.dropped_context_facts). - Add app/services/eval extraction-quality harness + extract_metrics + scripts/run_rca11_extract_eval.py; reference transcription fixture. - Revive app/services/eval (removed from the deleted-surfaces fence). Measured v1->v2 on Phys McatKing.pdf: 124 -> 51 facts, keyword-noise fraction 0.27 -> 0.065. mise run check green (666 passed). SPEC.md seeded (issue-scoped, B1 logged). Spun off RCA-12/13/14. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/services/eval/__init__.py | 37 ++++ app/services/eval/extract_metrics.py | 127 ++++++++++++ app/services/eval/metrics.py | 182 ++++++++++++++++++ app/services/kb/pdf_ingest.py | 162 +++++++++++++--- scripts/run_rca11_extract_eval.py | 144 ++++++++++++++ .../rca11_reference_transcriptions.json | 26 +++ tests/test_eval_extract_metrics.py | 76 ++++++++ tests/test_fence_guards.py | 4 +- tests/test_kb_inbox.py | 2 +- tests/test_kb_pdf_ingest.py | 142 ++++++++++++-- tests/test_pdf_ingest_api.py | 2 +- 11 files changed, 860 insertions(+), 44 deletions(-) create mode 100644 app/services/eval/__init__.py create mode 100644 app/services/eval/extract_metrics.py create mode 100644 app/services/eval/metrics.py create mode 100644 scripts/run_rca11_extract_eval.py create mode 100644 tests/fixtures/rca11_reference_transcriptions.json create mode 100644 tests/test_eval_extract_metrics.py diff --git a/app/services/eval/__init__.py b/app/services/eval/__init__.py new file mode 100644 index 0000000..c1a56d0 --- /dev/null +++ b/app/services/eval/__init__.py @@ -0,0 +1,37 @@ +"""Measurement harnesses — gate quality changes on recorded metrics (V-L2).""" + +from app.services.eval.extract_metrics import ( + ExtractDelta, + ExtractRunResult, + before_after_delta, + fact_yield, + is_noise, + keyword_noise_fraction, + summarize, +) +from app.services.eval.metrics import ( + EvalCase, + EvalReport, + EvalRunResult, + jaccard, + record_eval_run, + regression_blocks_pivot, +) + +__all__ = [ + # tagging-quality (V-L2 tagging gate) + "EvalCase", + "EvalReport", + "EvalRunResult", + "jaccard", + "record_eval_run", + "regression_blocks_pivot", + # extraction-quality (RCA-11) + "ExtractDelta", + "ExtractRunResult", + "before_after_delta", + "fact_yield", + "is_noise", + "keyword_noise_fraction", + "summarize", +] diff --git a/app/services/eval/extract_metrics.py b/app/services/eval/extract_metrics.py new file mode 100644 index 0000000..99b1713 --- /dev/null +++ b/app/services/eval/extract_metrics.py @@ -0,0 +1,127 @@ +"""Extraction-quality metric primitives for the RCA-11 measurement harness. + +Pure functions — no I/O, no LLM calls — so they unit-test cleanly. The live +runner (`scripts/run_rca11_extract_eval.py`) glues these to real extraction +calls over a snapshotted-transcription fixture. + +The headline question RCA-11 measures: did the doc-level learning-goals rework +cut lecture-context noise without gutting genuine fact yield? We can't automate +the human "is this memorizable?" judgment — but we can automate a *proxy*: the +fraction of kept facts that match lecture-context noise phrasing (the same kind +of crude keyword heuristic that flagged 33/124 in the issue). The runner dumps +the kept facts so a human makes the final call (V-L2 stays human-judged). +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from typing import Any + +# Crude lecture-context-noise heuristic. Each pattern matches phrasing typical of +# example/figure/meta prose rather than a durable, source-independent fact — +# e.g. "the page provides an example", "in the second example", "the text states", +# "shown in the diagram". This is a PROXY for noise, not ground truth: a low +# fraction is necessary, not sufficient, for quality. Human judgment is final. +_NOISE_PATTERNS: tuple[re.Pattern[str], ...] = ( + # The single strongest signal in the issue's sample: example-bound prose. + re.compile(r"\bexamples?\b", re.I), + re.compile(r"\bthe (text|page|slide|diagram|figure|author)\b", re.I), + re.compile(r"\bthis (problem|question|page|slide|figure|diagram)\b", re.I), + re.compile(r"\b(shown|depicted|illustrated|pictured|displayed|stated|mentioned) " + r"(in|on|above|below|here)\b", re.I), + re.compile(r"\b(is|are) (given|requested|asked|provided)\b", re.I), + re.compile(r"\b(it is stated|the problem asks|a common .* question)\b", re.I), +) + + +def is_noise(fact: str) -> bool: + """True if a fact matches the lecture-context-noise heuristic.""" + return any(p.search(fact) for p in _NOISE_PATTERNS) + + +def fact_yield(facts: list[str]) -> int: + """Number of facts produced.""" + return len(facts) + + +def keyword_noise_fraction(facts: list[str]) -> float: + """Fraction of facts matching the noise heuristic. Empty input ⇒ 0.0.""" + if not facts: + return 0.0 + return sum(1 for f in facts if is_noise(f)) / len(facts) + + +@dataclass(frozen=True) +class ExtractRunResult: + """Summary of one extraction pass over a document fixture. + + - `total` = every fact the model emitted (concept + context). + - `kept` = facts classified `concept` (what actually persists). + - `dropped_context` = facts classified `context` (dropped at persist, V-KB7). + - `noise_fraction` = `keyword_noise_fraction` over the KEPT facts (the proxy + that should fall toward ~0 after the rework). + """ + + label: str + total: int + kept: int + dropped_context: int + noise_fraction: float + + def as_dict(self) -> dict[str, Any]: + return { + "label": self.label, + "total": self.total, + "kept": self.kept, + "dropped_context": self.dropped_context, + "noise_fraction": round(self.noise_fraction, 4), + } + + +def summarize(label: str, facts: list[tuple[str, str]]) -> ExtractRunResult: + """Roll up `(text, kind)` pairs into an `ExtractRunResult`. + + `noise_fraction` is computed over the kept (`concept`) facts only — that's + the surface that pollutes the store and downstream tagging. + """ + kept = [text for text, kind in facts if kind == "concept"] + dropped = sum(1 for _, kind in facts if kind != "concept") + return ExtractRunResult( + label=label, + total=len(facts), + kept=len(kept), + dropped_context=dropped, + noise_fraction=keyword_noise_fraction(kept), + ) + + +@dataclass(frozen=True) +class ExtractDelta: + """Before/after comparison of two extraction passes.""" + + before: ExtractRunResult + after: ExtractRunResult + + @property + def yield_delta(self) -> int: + return self.after.total - self.before.total + + @property + def noise_delta(self) -> float: + return self.after.noise_fraction - self.before.noise_fraction + + def as_dict(self) -> dict[str, Any]: + return { + "before": self.before.as_dict(), + "after": self.after.as_dict(), + "yield_delta": self.yield_delta, + "noise_delta": round(self.noise_delta, 4), + } + + +def before_after_delta( + before: ExtractRunResult, after: ExtractRunResult +) -> ExtractDelta: + """Pair two runs for a diffable before/after report.""" + return ExtractDelta(before=before, after=after) diff --git a/app/services/eval/metrics.py b/app/services/eval/metrics.py new file mode 100644 index 0000000..a2e4cd7 --- /dev/null +++ b/app/services/eval/metrics.py @@ -0,0 +1,182 @@ +"""Metric primitives for the V-L2 measurement harness. + +Pure functions — no I/O, no LLM calls — so the harness can be unit-tested +end-to-end while the actual eval data sits in `tests/fixtures/`. Live runners +(`scripts/run_*_eval.py`) glue these to real LLM calls. + +Ported from the phase-3 worktree (tagging-quality gate); the extraction-quality +primitives for RCA-11 live alongside in `extract_metrics.py`. +""" + +from __future__ import annotations + +import json +import logging +import statistics +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + +# V-L2: tagging quality must not regress past the Claude baseline. The gate +# fails if the mean jaccard drops by more than `MEAN_JACCARD_TOLERANCE` OR +# set-equality drops by more than `SET_EQUALITY_TOLERANCE` (both in +# absolute percentage points). The tolerances are small but non-zero — a +# 1-point wobble on a sample-size of ~50 cases is noise. +MEAN_JACCARD_TOLERANCE = 0.03 +SET_EQUALITY_TOLERANCE = 0.05 + + +@dataclass(frozen=True) +class EvalCase: + """One evaluation row. + + - `qid` is opaque (e.g. `"Q1"`) — only used for logging. + - `gold_tags` is the canonical set of expected node_paths. + - `predicted_tags` is the set produced by the model under test. + """ + + qid: str + gold_tags: frozenset[str] + predicted_tags: frozenset[str] + + +@dataclass(frozen=True) +class EvalRunResult: + """Aggregate over an entire eval pass on one model.""" + + model: str + n_cases: int + mean_jaccard: float + set_equality_rate: float + per_case_jaccard: tuple[float, ...] + + def as_dict(self) -> dict[str, Any]: + return { + "model": self.model, + "n_cases": self.n_cases, + "mean_jaccard": round(self.mean_jaccard, 4), + "set_equality_rate": round(self.set_equality_rate, 4), + "per_case_jaccard": [round(j, 4) for j in self.per_case_jaccard], + } + + +@dataclass(frozen=True) +class EvalReport: + """Comparison of the model-under-test vs the Claude baseline.""" + + baseline: EvalRunResult + candidate: EvalRunResult + mean_jaccard_delta: float + set_equality_delta: float + blocks_pivot: bool + reason: str + + def as_dict(self) -> dict[str, Any]: + return { + "baseline": self.baseline.as_dict(), + "candidate": self.candidate.as_dict(), + "mean_jaccard_delta": round(self.mean_jaccard_delta, 4), + "set_equality_delta": round(self.set_equality_delta, 4), + "blocks_pivot": self.blocks_pivot, + "reason": self.reason, + } + + +def jaccard(a: frozenset[str], b: frozenset[str]) -> float: + """Jaccard similarity over two tag sets. Empty-on-both ⇒ 1.0 (perfect + agreement when there's nothing to disagree about).""" + if not a and not b: + return 1.0 + union = a | b + if not union: + return 1.0 + return len(a & b) / len(union) + + +def record_eval_run(model: str, cases: list[EvalCase]) -> EvalRunResult: + """Aggregate per-case jaccard + set-equality into a single run result.""" + if not cases: + return EvalRunResult( + model=model, + n_cases=0, + mean_jaccard=0.0, + set_equality_rate=0.0, + per_case_jaccard=(), + ) + per_case = tuple(jaccard(c.gold_tags, c.predicted_tags) for c in cases) + set_equality_rate = sum( + 1 for c in cases if c.gold_tags == c.predicted_tags + ) / len(cases) + return EvalRunResult( + model=model, + n_cases=len(cases), + mean_jaccard=statistics.fmean(per_case), + set_equality_rate=set_equality_rate, + per_case_jaccard=per_case, + ) + + +def regression_blocks_pivot( + baseline: EvalRunResult, + candidate: EvalRunResult, + *, + mean_jaccard_tolerance: float = MEAN_JACCARD_TOLERANCE, + set_equality_tolerance: float = SET_EQUALITY_TOLERANCE, +) -> EvalReport: + """V-L2: candidate must not drop more than the tolerance on either metric. + + Returns an `EvalReport` with `blocks_pivot=True` and a human-readable + `reason` when either delta exceeds the tolerance. + """ + mean_delta = candidate.mean_jaccard - baseline.mean_jaccard + set_delta = candidate.set_equality_rate - baseline.set_equality_rate + + reasons: list[str] = [] + if mean_delta < -mean_jaccard_tolerance: + reasons.append( + f"mean_jaccard dropped {mean_delta:+.3f} (tolerance " + f"-{mean_jaccard_tolerance:.3f})" + ) + if set_delta < -set_equality_tolerance: + reasons.append( + f"set_equality_rate dropped {set_delta:+.3f} (tolerance " + f"-{set_equality_tolerance:.3f})" + ) + + return EvalReport( + baseline=baseline, + candidate=candidate, + mean_jaccard_delta=mean_delta, + set_equality_delta=set_delta, + blocks_pivot=bool(reasons), + reason="; ".join(reasons) if reasons else "within tolerance", + ) + + +def load_baseline(path: Path) -> EvalRunResult: + """Read a baseline JSON file produced by `EvalRunResult.as_dict()`.""" + raw = json.loads(path.read_text()) + return EvalRunResult( + model=str(raw["model"]), + n_cases=int(raw["n_cases"]), + mean_jaccard=float(raw["mean_jaccard"]), + set_equality_rate=float(raw["set_equality_rate"]), + per_case_jaccard=tuple(float(x) for x in raw.get("per_case_jaccard", [])), + ) + + +def write_report(path: Path, report: EvalReport) -> None: + """Write the JSON report next to the script so it's diffable in PRs.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps(report.as_dict(), indent=2, sort_keys=True) + "\n", + encoding="utf-8", + ) + logger.info( + "V-L2 report written: blocks_pivot=%s reason=%s -> %s", + report.blocks_pivot, + report.reason, + path, + ) diff --git a/app/services/kb/pdf_ingest.py b/app/services/kb/pdf_ingest.py index 285ce0a..2d8246d 100644 --- a/app/services/kb/pdf_ingest.py +++ b/app/services/kb/pdf_ingest.py @@ -41,27 +41,63 @@ # Bump when the vision prompt / extraction schema changes meaningfully. # Stamped onto every persisted fact (V-KB3) so a re-run under a new version # is a clean miss once content_hash dedup is keyed differently downstream. -EXTRACTOR_VERSION = "pdf-vision-v1" +# v2 (RCA-11): document-level learning-goals synthesis + concept/context +# discernment gate + enriched vision (figure semantics). +EXTRACTOR_VERSION = "pdf-vision-v2" _RENDER_DPI = 150 _VISION_MAX_TOKENS = 4096 -_EXTRACT_MAX_TOKENS = 2048 +# Doc-level extraction (V-KB5): one call now covers the whole document, so the +# output budget is larger than the old per-page cap. +_EXTRACT_MAX_TOKENS = 3072 +# Concat-doc char ceiling before we split extraction on page boundaries (V-KB5). +# Keeps a multi-page document inside nano's context with room for the 3072 output. +_EXTRACT_MAX_DOC_CHARS = 48_000 _VISION_SYSTEM = ( "You transcribe a single page from a student's lecture notes or slide deck. " "The page may be typed, a slide image, or handwritten. Output the full " "readable text content of the page, faithfully and verbatim where legible. " - "Transcribe handwriting as best you can. Do NOT summarize, interpret, or add " - "commentary — emit only the page's own text. If the page has no legible text, " - "output nothing." + "Transcribe handwriting as best you can.\n" + "In ADDITION to the text, when the page contains a diagram, figure, graph, " + "chart, or table whose meaning is not fully captured by its text, emit a " + "brief factual description of it on its own line, prefixed `[figure]` " + "(e.g. `[figure] free-body diagram: block on an incline, friction vector " + "down-slope, gravity decomposed into components`). Describe only what is " + "shown — labels, relationships, quantities, structure. Do NOT add outside " + "knowledge, commentary, or interpretation beyond the visual. If the page has " + "no legible text and no informative figure, output nothing." ) _EXTRACT_SYSTEM = ( - "You extract atomic factual claims from a page of study material. " - "Each fact must be a single, self-contained, standalone statement that makes " - "sense without the surrounding text. Split compound sentences into separate " - "facts. Drop slide titles, page numbers, headers, and noise. Ground every " - "fact strictly in the provided text — do NOT invent or infer beyond it." + "You are given the full transcribed text of ONE study document (lecture notes " + "or a slide deck), with `=== page N ===` markers between pages. Produce the set " + "of atomic declarative facts a student is expected to LEARN from it.\n" + "\n" + "First, internally consider the document's learning goals: what key concepts, " + "definitions, principles, mechanisms, relationships, and quantitative laws " + "should a student understand after studying it? Then emit atomic facts that " + "COVER those goals.\n" + "\n" + "Each fact must be ONE self-contained declarative statement, true on its own " + "without the surrounding text. You MAY synthesize or restate a fact into clean " + "standalone form even when the document never phrases it as a single sentence " + "(summarization is allowed) — but stay grounded in the document's content and " + "do NOT introduce outside facts.\n" + "\n" + "Classify each fact with `kind`:\n" + "- `concept` — a durable, study-worthy declarative fact (definition, principle, " + "mechanism, relationship, quantitative law). This is the ONLY kind that is kept.\n" + "- `context` — describes a worked example, an example/practice-question setup, " + "what a problem asks, or one specific figure/diagram instance. Mark these " + "`context`, NOT `concept`.\n" + "\n" + "Do NOT emit at all (not even as context): slide titles, page numbers, headers, " + "author/course/date lines, and meta-statements about \"the text\", \"this page\", " + "\"the slide\", \"the diagram\", or \"the example below\".\n" + "\n" + "You produce declarative statements only. You must NEVER write a question, a " + "prompt, a flashcard, or any active-recall item — statements of fact only." ) @@ -84,9 +120,14 @@ class PageTranscription: cached_tokens: int = 0 +# A parsed fact: its text plus its discernment kind ('concept' kept, 'context' +# dropped at persist per V-KB7). +ParsedFact = tuple[str, str] + + @dataclass class FactExtraction: - facts: list[str] = field(default_factory=list) + facts: list[ParsedFact] = field(default_factory=list) prompt_tokens: int = 0 output_tokens: int = 0 cached_tokens: int = 0 @@ -100,6 +141,9 @@ class IngestReport: pages: int reused_pdf: bool extractor_version: str = EXTRACTOR_VERSION + # V-KB7: example/lecture-context facts the model flagged 'context' and we + # dropped before persist. + dropped_context_facts: int = 0 # V-L1: token accounting summed across every vision + extraction call. input_tokens: int = 0 output_tokens: int = 0 @@ -219,7 +263,7 @@ async def transcribe_page( "type": "json_schema", "json_schema": { "name": "extract_atomic_facts", - "description": "Atomic factual claims grounded in the page text.", + "description": "Atomic declarative facts grounded in the document text.", "strict": True, "schema": { "type": "object", @@ -230,12 +274,20 @@ async def transcribe_page( "type": "array", "items": { "type": "object", - "required": ["text"], + "required": ["text", "kind"], "additionalProperties": False, "properties": { "text": { "type": "string", - "description": "One self-contained atomic claim.", + "description": "One self-contained atomic declarative claim.", + }, + "kind": { + "type": "string", + "enum": ["concept", "context"], + "description": ( + "concept = durable study-worthy fact (kept); " + "context = example/figure/problem prose (dropped)." + ), }, }, }, @@ -246,8 +298,8 @@ async def transcribe_page( } -def _parse_facts(payload: dict[str, Any]) -> list[str]: - out: list[str] = [] +def _parse_facts(payload: dict[str, Any]) -> list[ParsedFact]: + out: list[ParsedFact] = [] for raw in payload.get("facts") or []: if not isinstance(raw, dict): continue @@ -255,8 +307,14 @@ def _parse_facts(payload: dict[str, Any]) -> list[str]: if not isinstance(text, str): continue text = text.strip() - if text: - out.append(text) + if not text: + continue + # V-KB7: default to 'concept' on a missing/invalid kind — never silently + # drop a fact because the discriminator field was malformed. + kind = raw.get("kind") + if kind not in ("concept", "context"): + kind = "concept" + out.append((text, kind)) return out @@ -268,10 +326,11 @@ async def extract_atomic_facts( max_tokens: int = _EXTRACT_MAX_TOKENS, service_tier: str | None = None, ) -> FactExtraction: - """One OpenAI structured-output call: page text → atomic facts (V-KB4). + """One OpenAI structured-output call: document text → atomic facts (V-KB4). - Empty/blank input → no LLM call, empty result. Strict json_schema emits the - document in ``choice.message.content`` (mirrors ``llm/grounded.py``). + ``text`` is the full document (page transcriptions concatenated, V-KB5), not a + single page. Empty/blank input → no LLM call, empty result. Strict json_schema + emits the document in ``choice.message.content`` (mirrors ``llm/grounded.py``). ``service_tier`` (e.g. ``'flex'``, V-L5) is forwarded when set.""" if not text.strip(): @@ -292,7 +351,7 @@ async def extract_atomic_facts( prompt_tokens, output_tokens, cached_tokens = _read_usage(completion) content = _message_content(completion) - facts: list[str] = [] + facts: list[ParsedFact] = [] if content: try: facts = _parse_facts(json.loads(content)) @@ -311,6 +370,42 @@ async def extract_atomic_facts( # --------------------------------------------------------------------------- # +def _format_page(page: int, text: str) -> str: + return f"=== page {page} ===\n{text}" + + +def _chunk_document( + transcribed: list[tuple[int, str]], + *, + max_chars: int = _EXTRACT_MAX_DOC_CHARS, +) -> list[tuple[int, str]]: + """Group page transcriptions into document chunks for extraction (V-KB5). + + Pages are concatenated with ``=== page N ===`` markers into as few chunks as + fit under ``max_chars``; splits fall only on page boundaries. Each chunk is + returned with its lead (first) page, used to stamp ``AtomicFact.page``. A + single page longer than ``max_chars`` becomes its own chunk (never split + mid-page). Empty input → no chunks (no extraction call).""" + + chunks: list[tuple[int, str]] = [] + cur: list[str] = [] + cur_lead = 0 + cur_len = 0 + for page, text in transcribed: + block = _format_page(page, text) + sep = 2 if cur else 0 # cost of the '\n\n' join + if cur and cur_len + sep + len(block) > max_chars: + chunks.append((cur_lead, "\n\n".join(cur))) + cur, cur_lead, cur_len, sep = [], 0, 0, 0 + if not cur: + cur_lead = page + cur.append(block) + cur_len += sep + len(block) + if cur: + chunks.append((cur_lead, "\n\n".join(cur))) + return chunks + + def _sha256_file(path: Path) -> str: h = hashlib.sha256() with open(path, "rb") as f: @@ -382,11 +477,14 @@ async def ingest_pdf( pages = renderer(path) new_facts = 0 dup_facts = 0 + dropped_context = 0 in_tokens = 0 out_tokens = 0 cached = 0 seen_hashes: set[str] = set() + # Pass 1 — transcribe every page (vision is inherently per-image, V-KB8). + transcribed: list[tuple[int, str]] = [] for rendered in pages: transcription = await transcribe_page( rendered.image_png, @@ -397,9 +495,14 @@ async def ingest_pdf( in_tokens += transcription.prompt_tokens out_tokens += transcription.output_tokens cached += transcription.cached_tokens + if transcription.text: + transcribed.append((rendered.page, transcription.text)) + # Pass 2 — document-level extraction (V-KB5): concat page transcriptions and + # run one extraction per chunk so the model sees the whole learning arc. + for chunk_lead_page, chunk_text in _chunk_document(transcribed): extraction = await extract_atomic_facts( - transcription.text, + chunk_text, client=extract_client, model=resolved_extract_model, service_tier=service_tier, @@ -408,7 +511,12 @@ async def ingest_pdf( out_tokens += extraction.output_tokens cached += extraction.cached_tokens - for fact_text in extraction.facts: + for fact_text, kind in extraction.facts: + # V-KB7: discernment gate — only durable 'concept' facts persist; + # 'context' (example/figure/problem prose) is dropped and counted. + if kind != "concept": + dropped_context += 1 + continue content_hash = _sha256_text(fact_text) if content_hash in seen_hashes: dup_facts += 1 @@ -429,7 +537,7 @@ async def ingest_pdf( AtomicFact( course_id=course_id, pdf_source_id=pdf_id, - page=rendered.page, + page=chunk_lead_page, text=fact_text, content_hash=content_hash, extractor_version=extractor_version, @@ -443,12 +551,13 @@ async def ingest_pdf( await session.flush() _logger.info( - "pdf_ingest: pdf=%d pages=%d new_facts=%d dup_facts=%d " + "pdf_ingest: pdf=%d pages=%d new_facts=%d dup_facts=%d dropped_context=%d " "vision_model=%s extract_model=%s prompt=%d cached=%d out=%d version=%s", pdf_id, len(pages), new_facts, dup_facts, + dropped_context, resolved_vision_model, resolved_extract_model, in_tokens, @@ -464,6 +573,7 @@ async def ingest_pdf( pages=len(pages), reused_pdf=False, extractor_version=extractor_version, + dropped_context_facts=dropped_context, input_tokens=in_tokens, output_tokens=out_tokens, cached_tokens=cached, diff --git a/scripts/run_rca11_extract_eval.py b/scripts/run_rca11_extract_eval.py new file mode 100644 index 0000000..9559f9d --- /dev/null +++ b/scripts/run_rca11_extract_eval.py @@ -0,0 +1,144 @@ +"""RCA-11 extraction-quality measurement harness (V-L2). + +Measures whether the document-level learning-goals rework (pdf-vision-v2) cuts +lecture-context noise without gutting genuine fact yield. Two modes: + + # 1. Capture the reference transcriptions ONCE (one real vision pass per page) + python -m scripts.run_rca11_extract_eval \\ + --pdf "/Users/rcantu/Desktop/Phys McatKing.pdf" \\ + --save-fixture tests/fixtures/rca11_reference_transcriptions.json + + # 2. Replay extraction over saved transcriptions (cheap, deterministic — no + # vision, no PyMuPDF, no DB; isolates the one changed variable) + python -m scripts.run_rca11_extract_eval \\ + --fixture tests/fixtures/rca11_reference_transcriptions.json \\ + --report data/rca11_extract_report.json + +Both modes run document-level extraction (concat → chunk → extract per chunk), +print yield / kept / dropped-context / keyword-noise-fraction, and DUMP the kept +facts to stdout for the human "memorizable?" judgment — the gate stays +human-judged (cognitive-safety: AI is not the arbiter of recall-worthiness). + +Lives outside `app/` so it never imports into production paths. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import logging +import sys +from pathlib import Path +from typing import Any + +from app.config import settings +from app.services.eval.extract_metrics import summarize +from app.services.kb import pdf_ingest +from app.services.llm.client import build_openai_client + +logger = logging.getLogger("rca11_extract_eval") + + +async def _capture_transcriptions(pdf_path: Path, *, client: Any) -> list[dict[str, Any]]: + """One real vision pass per page → `[{page, text}]` (the saved fixture).""" + service_tier = settings.OPENAI_SERVICE_TIER + model = settings.OPENAI_VISION_MODEL or settings.OPENAI_MODEL + pages = pdf_ingest.render_pages(pdf_path) + out: list[dict[str, Any]] = [] + for rendered in pages: + t = await pdf_ingest.transcribe_page( + rendered.image_png, client=client, model=model, service_tier=service_tier + ) + out.append({"page": rendered.page, "text": t.text}) + logger.info("transcribed page %d (%d chars)", rendered.page, len(t.text)) + return out + + +def _load_transcriptions(path: Path) -> list[tuple[int, str]]: + raw = json.loads(path.read_text()) + return [(int(r["page"]), str(r["text"])) for r in raw if str(r.get("text", "")).strip()] + + +async def _extract_document( + transcribed: list[tuple[int, str]], *, client: Any +) -> list[tuple[str, str]]: + """Document-level extraction over the same chunking the ingest path uses.""" + service_tier = settings.OPENAI_SERVICE_TIER + model = settings.OPENAI_MODEL + facts: list[tuple[str, str]] = [] + chunks = pdf_ingest._chunk_document(transcribed) + for lead_page, chunk_text in chunks: + extraction = await pdf_ingest.extract_atomic_facts( + chunk_text, client=client, model=model, service_tier=service_tier + ) + logger.info("extracted %d facts from chunk lead-page %d", len(extraction.facts), lead_page) + facts.extend(extraction.facts) + return facts + + +async def run(args: argparse.Namespace) -> int: + logging.basicConfig(level=logging.INFO, format="%(message)s") + client = build_openai_client(max_retries=5) + + if args.pdf: + transcriptions = await _capture_transcriptions(Path(args.pdf), client=client) + if args.save_fixture: + out = Path(args.save_fixture) + out.parent.mkdir(parents=True, exist_ok=True) + out.write_text(json.dumps(transcriptions, indent=2) + "\n", encoding="utf-8") + print(f"saved {len(transcriptions)} page transcriptions -> {out}") + transcribed = [ + (int(r["page"]), str(r["text"])) + for r in transcriptions + if str(r.get("text", "")).strip() + ] + elif args.fixture: + transcribed = _load_transcriptions(Path(args.fixture)) + else: + raise SystemExit("pass --pdf (capture) or --fixture (replay)") + + if not transcribed: + raise SystemExit("no non-empty transcriptions to extract from") + + facts = await _extract_document(transcribed, client=client) + result = summarize(settings.OPENAI_MODEL, facts) + + print( + f"\n=== RCA-11 extract eval ({pdf_ingest.EXTRACTOR_VERSION}, " + f"model={settings.OPENAI_MODEL}) ===\n" + f"pages={len(transcribed)} total_facts={result.total} " + f"kept(concept)={result.kept} dropped(context)={result.dropped_context} " + f"keyword_noise_fraction(kept)={result.noise_fraction:.3f}\n" + ) + print("--- kept facts (human 'memorizable?' judgment) ---") + for text, kind in facts: + if kind == "concept": + print(f" • {text}") + + if args.report: + rep = Path(args.report) + rep.parent.mkdir(parents=True, exist_ok=True) + payload = { + "extractor_version": pdf_ingest.EXTRACTOR_VERSION, + "result": result.as_dict(), + "kept_facts": [t for t, k in facts if k == "concept"], + "dropped_facts": [t for t, k in facts if k != "concept"], + } + rep.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8") + print(f"\nreport -> {rep}") + return 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="RCA-11 extraction-quality harness (V-L2)") + parser.add_argument("--pdf", help="capture mode: PDF to vision-transcribe once") + parser.add_argument("--save-fixture", help="where to write captured transcriptions (with --pdf)") + parser.add_argument("--fixture", help="replay mode: saved transcriptions JSON") + parser.add_argument("--report", help="optional JSON report path") + args = parser.parse_args(argv) + return asyncio.run(run(args)) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/fixtures/rca11_reference_transcriptions.json b/tests/fixtures/rca11_reference_transcriptions.json new file mode 100644 index 0000000..7af2b78 --- /dev/null +++ b/tests/fixtures/rca11_reference_transcriptions.json @@ -0,0 +1,26 @@ +[ + { + "page": 1, + "text": "Atoms\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a018 Jan\n\n# Protons + # neutrons\n\nAtomic #\u2019s is #p and gives atoms their identity.\n\nIsotopes are useful in image, this is known as isotope labeling \nor \nisotopic labeling\n\nIsotopes\n\nall carbon isotopes have same chemical properties, but different physical properties. (mass, interaction with light)\n\nNucleus\n\nprotons provide positive charge \nneutrons serve as cushioning to stabilize proton-proton repulsions in the nucleus. they also provide extra mass.\n\nNucleus proton ratio N/p > 1 except for hydrogen\n\nhydrogen does not have a neutron.\n\n[figure] small schematic showing atomic number notation beside X, and an isotope branching diagram labeled \u201cIsotopes\u201d leading to \u00b9\u00b2\u2086C and \u00b9\u00b3\u2086C" + }, + { + "page": 2, + "text": "outside nucleus\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u2003\u200318 Jan\n\nphotons \nheat/work \natom \nE = hc/\u03bb = h\u03bd \n\nKE = 1/2 mv\u00b2\n\nKE(e\u2082) > KE(e\u2081)\n\nelectrons in further shells have higher KE and move faster. \n\u2234 \nIt takes energy to excite electrons into a higher shell.\n\nHow to transfer energy\n\nheat: \n\u0394E = Q + W \n\u0394E = Q + W \nQ = mc\u0394T \nQ = m\u0394H\n\nwork: \nW = F \u00b7 \u0394X\n\nLight/photons\n\nPeriod (T), time to orbit around emma \nfrequency, number of orbits per sec. \n\nf = \u03bd = 1/T\u2003\u2003units \nsec\u207b\u00b9 or Hz. \n\u03bd, nu is a common script in physics context\n\nVelocity\n\nV = x/t\u2003or\u2003d/t\n\nvelocity of an orbit.\n\nV = circumference/T = 2\u03c0r/T = revolution/T = \u03bb/T = \u03bb\u03bd_t\n\n\u03bb, wavelength.\n\nV = \u03bb\u03bd_t \nmemorize based equation.\n\nRoy G B V \nlong \u03bb \nshort \u03bb\n\nvelocity of light waves is dictated by medium \nin vacuum v = c = 3 \u00d7 10\u2078 m/s\n\nas \u03bb \u2191, \u03bd is const, \u03bb decreases.\n\n\u03bd_t = c/\u03bb \nE = h\u03bd_t \nE = hc/\u03bb \nhc = 1240 eV nm\n\nh = 6 \u00d7 10\u207b\u00b3\u2074 Js\n\nHomework \non-demand, Mech syllabus\u2003class #1 GDRF? \nWednesday, no class, warm up y \nLec 1 | Kinematic Eq's\n\nL Read Ch 2/3 Kaplan. \nthen back. \nRead as 2nd reading.\n\nstart Bio book\n\neveryday Cars passages." + }, + { + "page": 3, + "text": "Kine matics\n\nd = 3 m \nt = 1 s\n\nv\u2080 = 0 \na = g = 10 m/s\u00b2 \nd = ? \nv\u1d62 = ?\n\nequation: V = V\u2080 + at => \u2717 \nv\u00b2 - v\u2080\u00b2 = 2ad => \u2717 \nd = V\u2080t + 1/2 at\u00b2\n\nd = 15 m \nt = 2 s\n\nV = 10 m/s\u00b2 \u00b7 (1 s) = 10 m/s \nd = 1/2 (10 m/s\u00b2) = 5 m\n\nd = 45 m \nt = 3 s\n\nV(2s) = 10 m/s\u00b2 (2s) = 20 m/s \nd = 1/2 (10 m/s\u00b2) \u00b7 (2s)\u00b2 = 20 m\n\nDO 150 Q an EK Phys. \nfree fall question can be answered with \nhand jt.\n\nex:\n\nV\u2080 = 50 m/s \nflight time is going to be 5 s to \nreach 0 m and 5 s to reach ground.\n\ndistance is 45 + 35 + 25 + 15 + 5 \ndoubled.\n\n[figure] handwritten kinematics notes with three small vertical motion sketches on the left showing distances d = 3 m, d = 15 m, and d = 45 m over times t = 1 s, t = 2 s, and t = 3 s; a larger hand-drawn curved path/loop diagram labeled with numbers like 40, 35, 25, 15, 5; and a small arrow-up sketch beside the example with V\u2080 = 50 m/s." + }, + { + "page": 4, + "text": "t=3s\n\nIn 3 s ball travels : 45 m\n\nh = ?\n45 m = h\n\n1 s | 5 \n2 s | 15 \n3 s | 25\n\nbut 3 s is time of ball hitting ground + time for sound to travl \nup.\n\nV_sound = 340 m/s , time for sound is negligible.\n\nEx 2:\n\na = -10 m/s \nv\u1d62 = 40 m/s\n\nd = ? 80 m\n\nV=0 m/s @ t=4s.\n\n[figure] hand-drawn sketch of a ball falling from a ledge into a vertical shaft, with a downward arrow indicating motion and height labeled h; lower sketch of a car moving to the right toward a person, with a long horizontal distance labeled 80 m and notes for initial velocity, acceleration, and final velocity." + }, + { + "page": 5, + "text": "Projective motion\n\nv = 60 m/s\n\n1) break velocity into components\n\nVx\u2080 = V\u2080 \u00b7 cos(30\u00b0) = 60 cos(30\u00b0) \nVy\u2080 = V\u2080 \u00b7 sin(30\u00b0) = 60 sin(30\u00b0)\n\nUnit circle:\n\n\u03c0/2 90\u00b0 (0,1) \n\u03c0/3 60\u00b0 \n\u03c0/4 45\u00b0 \n\u03c0/6 30\u00b0 \n0\u00b0 (1,0)\n\n(1/2, \u221a3/2) \n(\u221a2/2, \u221a2/2) \n(\u221a3/2, 1/2)\n\nVx\u2080 = 60 \u00b7 \u221a3/2 \u2248 52 m/s \nVy = 60 \u00b7 1/2 = 30 m/s\n\n2) think of velocity comp. independently.\n\nVy | max hight, Vy = 0 . t = 3 s , time of flight t = 6.\n\nh(3s) = 45 m \u201cmax height\u201d\n\nVx | range = ?\n\nd = Vx \u00b7 t = 52 m/s \u00b7 6s = 312 m\n\nFormula example\n\nVy | calc. hmax.\n\nVyt = 0 => Vy\u2080\u00b2 = 2 \u00b7 (10 m/s\u00b2) \u00b7 h\n\nVyt\u00b2 \u2212 Vyt\u00b2 = 2 a \u00b7 d\n\nh = Vy\u2080\u00b2 / 2(10 m/s\u00b2) = (30)\u00b2 / 20 = 900/20 = 45 m\n\nEnergy Example\n\nWhy cant we use ?\n\nE1 = E2\n\nPE\u2081 + KE\u2081 = PE\u2082 + KE\u2082\n\n@ E1, KE = 1/2 m v\u00b2, PE = 0 \n@ E2, KE = 1/2 m v\u00b2, PE = mgh\n\nstudents forget that KE still has energy in it. \nVx component!\n\nWhat does work:\n\n1/2 mVy\u2080\u00b2 + 1/2 mVy\u2080\u00b2 = mghmax + 1/2 mVx\u00b2\n\nKEx cancels out.\n\n[figure] projectile-motion sketches and a unit-circle diagram: small red launch vector at 30\u00b0 above horizontal; right-triangle component sketch; unit-circle axes with angle markers 30\u00b0, 45\u00b0, 60\u00b0, 90\u00b0 and coordinate labels; curved projectile arc from E1 on the left to E2 at the top and down to the right, with a dashed vertical line through the apex." + }, + { + "page": 6, + "text": "\u00bdat\u00b2\n\nCommon projectile motion question:\n\nV\u20d7\u2080 = 20 \u00ee m/s + 0 \u0135 m/s\n\nfind distance from table.\n\n1) find T, F using h and Jb \n2) Vx \u00b7 T\u2080, F = d.\n\n[figure] sketch of a projectile launched horizontally from a table, with a red horizontal distance arrow labeled d and several dotted trajectory points to the right of the table\n\nEx:\n\nA projectile is launched at angle 30\u00b0 to the horizontal from a 15 m platform. Its initial velocity is 20 m/s. How far does it travel?\n\nV = 20 m/s \nh = 15 m\n\nVy = 20\u00b7sin(30\u00b0) = 20\u00b7(1/2) = 10 m/s\n\nT\u2080F = 1 s + 2 s = 3 s \nat Hmax \nat h = 0\n\nh max = 20 m\n\nVx = 20\u00b7(\u221a3/2) = 10\u221a3\n\nd = 10\u221a3 \u00b7 3 s \u2248 17 m/s \u00b7 3 s \u2248 51 m" + } +] diff --git a/tests/test_eval_extract_metrics.py b/tests/test_eval_extract_metrics.py new file mode 100644 index 0000000..9ee3b0b --- /dev/null +++ b/tests/test_eval_extract_metrics.py @@ -0,0 +1,76 @@ +"""RCA-11 extraction-quality metric primitives (pure, no LLM/I/O).""" + +from __future__ import annotations + +from app.services.eval.extract_metrics import ( + before_after_delta, + fact_yield, + is_noise, + keyword_noise_fraction, + summarize, +) + +# Real noise emitted by pdf-vision-v1 (from the RCA-11 issue) — must flag. +_NOISE = [ + "The page provides an example with V0 = 50 m/s.", + "In the second example, the distance d is requested.", + "The text states that a student might forget that KE still has energy in it.", + "A formula example for maximum height uses Vyf = 0.", +] +# Durable, source-independent facts — must NOT flag. +_CLEAN = [ + "Force equals mass times acceleration.", + "Kinetic energy equals one half m v squared.", + "Acceleration due to gravity near Earth's surface is 9.8 m/s^2.", +] + + +def test_is_noise_flags_lecture_context_phrasing(): + assert all(is_noise(f) for f in _NOISE) + + +def test_is_noise_passes_durable_facts(): + assert not any(is_noise(f) for f in _CLEAN) + + +def test_fact_yield_counts(): + assert fact_yield(_CLEAN) == 3 + assert fact_yield([]) == 0 + + +def test_keyword_noise_fraction(): + assert keyword_noise_fraction([]) == 0.0 + assert keyword_noise_fraction(_CLEAN) == 0.0 + assert keyword_noise_fraction(_NOISE) == 1.0 + # 1 noise of 4 → 0.25 + assert keyword_noise_fraction(_CLEAN + _NOISE[:1]) == 0.25 + + +def test_summarize_splits_concept_and_context(): + facts = [(t, "concept") for t in _CLEAN] + [(t, "context") for t in _NOISE] + res = summarize("model-x", facts) + assert res.total == 7 + assert res.kept == 3 + assert res.dropped_context == 4 + # noise_fraction is over KEPT facts only — all kept are clean. + assert res.noise_fraction == 0.0 + + +def test_summarize_noise_over_kept_only(): + # A noisy fact mislabeled 'concept' still counts against the kept noise frac. + facts = [ + ("Force equals mass times acceleration.", "concept"), + ("The page provides an example with V0 = 50 m/s.", "concept"), + ] + res = summarize("model-x", facts) + assert res.kept == 2 + assert res.noise_fraction == 0.5 + + +def test_before_after_delta(): + before = summarize("v1", [(t, "concept") for t in _CLEAN + _NOISE]) + after = summarize("v2", [(t, "concept") for t in _CLEAN]) + delta = before_after_delta(before, after) + assert delta.yield_delta == -4 + assert delta.noise_delta < 0 # noise fraction fell + assert delta.as_dict()["before"]["kept"] == 7 diff --git a/tests/test_fence_guards.py b/tests/test_fence_guards.py index 3e86e24..7318818 100644 --- a/tests/test_fence_guards.py +++ b/tests/test_fence_guards.py @@ -206,7 +206,9 @@ def test_scheduler_feature_extraction_absent(): "app.services.recommender", "app.services.topic_subtree", "app.services.analyzer", - "app.services.eval", + # NOTE: app.services.eval was deleted in T53 (legacy MCAT eval) but + # revived for RCA-11 as the extraction-quality measurement harness + # (V-L2). It is a live package again — no longer a fenced surface. "app.services.categorizer", "app.services.llm.batch", "app.api.v1.analyzer", diff --git a/tests/test_kb_inbox.py b/tests/test_kb_inbox.py index a100506..cefdf5e 100644 --- a/tests/test_kb_inbox.py +++ b/tests/test_kb_inbox.py @@ -69,7 +69,7 @@ async def test_poll_ingests_per_slug(db_session: AsyncSession, tmp_path: Path): facts = (await db_session.execute(select(AtomicFact))).scalars().all() assert len(facts) == 1 - assert facts[0].extractor_version == "pdf-vision-v1" + assert facts[0].extractor_version == "pdf-vision-v2" assert facts[0].node_id is None diff --git a/tests/test_kb_pdf_ingest.py b/tests/test_kb_pdf_ingest.py index 8cd7307..107dd27 100644 --- a/tests/test_kb_pdf_ingest.py +++ b/tests/test_kb_pdf_ingest.py @@ -26,6 +26,7 @@ from app.models.outline import Course from app.models.pdf_source import PdfSource from app.services.kb.pdf_ingest import ( + _EXTRACT_MAX_DOC_CHARS, EXTRACTOR_VERSION, FactExtraction, IngestReport, @@ -57,8 +58,16 @@ def _renderer(_: Path) -> list[RenderedPage]: def _facts_completion(*facts: str, **kw): - """A structured-output completion whose JSON body lists ``facts``.""" - body = json.dumps({"facts": [{"text": f} for f in facts]}) + """A structured-output completion whose JSON body lists ``facts``; every + fact defaults to ``kind='concept'`` (the kept kind, V-KB7).""" + body = json.dumps({"facts": [{"text": f, "kind": "concept"} for f in facts]}) + return make_completion(content=body, **kw) + + +def _facts_completion_kinds(pairs: list[tuple[str, str]], **kw): + """A completion with explicit ``(text, kind)`` pairs — for mixed + concept/context discernment-gate tests (V-KB7).""" + body = json.dumps({"facts": [{"text": t, "kind": k} for t, k in pairs]}) return make_completion(content=body, **kw) @@ -89,7 +98,27 @@ async def test_extract_atomic_facts_parses_structured_output(): client = make_client(_facts_completion("Fact one is here.", "Fact two is here.")) out = await extract_atomic_facts("some page text", client=client, model="gpt-4.1-mini") assert isinstance(out, FactExtraction) - assert out.facts == ["Fact one is here.", "Fact two is here."] + assert out.facts == [("Fact one is here.", "concept"), ("Fact two is here.", "concept")] + + +async def test_extract_atomic_facts_carries_kind(): + """V-KB7: each fact carries its concept/context discernment kind.""" + client = make_client( + _facts_completion_kinds([("A durable fact.", "concept"), ("An example setup.", "context")]) + ) + out = await extract_atomic_facts("text", client=client, model="gpt-4.1-mini") + assert out.facts == [("A durable fact.", "concept"), ("An example setup.", "context")] + + +async def test_extract_defaults_malformed_kind_to_concept(): + """V-KB7: a missing/invalid kind defaults to 'concept' — never silently + dropped because the discriminator field was malformed.""" + body = json.dumps( + {"facts": [{"text": "No kind field."}, {"text": "Bad kind.", "kind": "bogus"}]} + ) + client = make_client(make_completion(content=body)) + out = await extract_atomic_facts("text", client=client, model="gpt-4.1-mini") + assert out.facts == [("No kind field.", "concept"), ("Bad kind.", "concept")] async def test_vision_and_extract_forward_service_tier(): @@ -113,10 +142,12 @@ async def test_extract_atomic_facts_blank_text_skips_llm_call(): async def test_extract_atomic_facts_drops_empty_and_nonstring(): - body = json.dumps({"facts": [{"text": " Kept fact. "}, {"text": " "}, {"text": 5}]}) + body = json.dumps( + {"facts": [{"text": " Kept fact. ", "kind": "concept"}, {"text": " "}, {"text": 5}]} + ) client = make_client(make_completion(content=body)) out = await extract_atomic_facts("text", client=client, model="gpt-4.1-mini") - assert out.facts == ["Kept fact."] + assert out.facts == [("Kept fact.", "concept")] # --------------------------------------------------------------------------- # @@ -133,7 +164,8 @@ async def test_first_ingest_renders_transcribes_extracts(db_session: AsyncSessio renderer = _forge_renderer( [RenderedPage(page=1, image_png=b"png-1"), RenderedPage(page=2, image_png=b"png-2")] ) - # Two pages → two vision calls (transcription) + two extraction calls. + # Two pages → two vision calls (transcription) + ONE document-level + # extraction call (V-KB5: page transcriptions concat into one chunk). vision_client = make_client( make_completion(content="page one text", prompt_tokens=1000, completion_tokens=100), make_completion(content="page two text", prompt_tokens=1000, completion_tokens=100), @@ -142,14 +174,10 @@ async def test_first_ingest_renders_transcribes_extracts(db_session: AsyncSessio _facts_completion( "Glycolysis converts glucose to pyruvate.", "It yields net two ATP per glucose.", - prompt_tokens=500, - completion_tokens=80, - cached_tokens=10, - ), - _facts_completion( "The TCA cycle regenerates oxaloacetate.", prompt_tokens=500, completion_tokens=80, + cached_tokens=10, ), ) @@ -167,10 +195,14 @@ async def test_first_ingest_renders_transcribes_extracts(db_session: AsyncSessio assert report.pages == 2 assert report.new_facts == 3 assert report.dup_facts == 0 + assert report.dropped_context_facts == 0 assert report.extractor_version == EXTRACTOR_VERSION - # V-L1: tokens summed across all 4 calls (vision 2×1000 + extract 2×500). - assert report.input_tokens == 3000 - assert report.output_tokens == 360 + # V-KB5: vision is per-page (2 calls); extraction is document-level (1 call). + assert vision_client.chat.completions.create.await_count == 2 + assert extract_client.chat.completions.create.await_count == 1 + # V-L1: tokens summed across all 3 calls (vision 2×1000 + extract 1×500). + assert report.input_tokens == 2500 + assert report.output_tokens == 280 assert report.cached_tokens == 10 pdf_row = ( @@ -190,12 +222,92 @@ async def test_first_ingest_renders_transcribes_extracts(db_session: AsyncSessio .all() ) assert len(facts) == 3 - assert {f.page for f in facts} == {1, 2} + # V-KB5: both pages concat into one chunk → stamped with the chunk lead page. + assert {f.page for f in facts} == {1} # V-KB4: node_id NULL until the categorizer (T50) runs; V-KB3: version stamped. assert all(f.node_id is None for f in facts) assert all(f.extractor_version == EXTRACTOR_VERSION for f in facts) +async def test_context_kind_dropped_at_persist(db_session: AsyncSession, tmp_path: Path): + """V-KB7: facts the model flags 'context' are dropped before persist and + counted in ``dropped_context_facts``; only 'concept' rows land in the DB.""" + course = await _make_course(db_session) + course_id = course.id + pdf = tmp_path / "mixed.pdf" + _fake_pdf(pdf, b"%PDF-1.4 mixed") + renderer = _forge_renderer([RenderedPage(page=1, image_png=b"png-1")]) + + report = await ingest_pdf( + db_session, + course_id=course_id, + path=pdf, + vision_client=make_client(make_completion(content="page text")), + extract_client=make_client( + _facts_completion_kinds( + [ + ("Force equals mass times acceleration.", "concept"), + ("Kinetic energy is one half m v squared.", "concept"), + ("The page provides an example with V0 = 50 m/s.", "context"), + ] + ) + ), + renderer=renderer, + ) + + assert report.new_facts == 2 + assert report.dropped_context_facts == 1 + assert report.dup_facts == 0 + + rows = ( + ( + await db_session.execute( + select(AtomicFact).where(AtomicFact.pdf_source_id == report.pdf_source_id) + ) + ) + .scalars() + .all() + ) + assert len(rows) == 2 + texts = {r.text for r in rows} + assert "The page provides an example with V0 = 50 m/s." not in texts + + +async def test_extraction_chunks_large_document(db_session: AsyncSession, tmp_path: Path): + """V-KB5: when concatenated transcriptions exceed the char ceiling, extraction + splits on page boundaries — more than one extract call, results unioned.""" + course = await _make_course(db_session) + course_id = course.id + pdf = tmp_path / "big.pdf" + _fake_pdf(pdf, b"%PDF-1.4 big") + renderer = _forge_renderer( + [RenderedPage(page=1, image_png=b"png-1"), RenderedPage(page=2, image_png=b"png-2")] + ) + # Each page transcription alone is under the ceiling, but the two together + # exceed it → two chunks → two extraction calls. + big_text = "x" * (_EXTRACT_MAX_DOC_CHARS - 100) + vision_client = make_client( + make_completion(content=big_text), + make_completion(content=big_text), + ) + extract_client = make_client( + _facts_completion("Fact from chunk one."), + _facts_completion("Fact from chunk two."), + ) + + report = await ingest_pdf( + db_session, + course_id=course_id, + path=pdf, + vision_client=vision_client, + extract_client=extract_client, + renderer=renderer, + ) + + assert extract_client.chat.completions.create.await_count == 2 + assert report.new_facts == 2 + + async def test_re_ingest_same_file_is_noop(db_session: AsyncSession, tmp_path: Path): course = await _make_course(db_session) course_id = course.id diff --git a/tests/test_pdf_ingest_api.py b/tests/test_pdf_ingest_api.py index 3f71e45..8ffd397 100644 --- a/tests/test_pdf_ingest_api.py +++ b/tests/test_pdf_ingest_api.py @@ -64,7 +64,7 @@ async def test_pdf_ingest_happy_path(client: AsyncClient, db_session: AsyncSessi assert body["pages"] == 1 assert body["new_facts"] == 1 assert body["reused_pdf"] is False - assert body["extractor_version"] == "pdf-vision-v1" + assert body["extractor_version"] == "pdf-vision-v2" async def test_pdf_ingest_unknown_course_404(client: AsyncClient): From b7d1089474e18a4a3238e0029d9470493c466325 Mon Sep 17 00:00:00 2001 From: Ramiro Cantu Date: Mon, 1 Jun 2026 11:13:39 -0500 Subject: [PATCH 2/2] fix(eval): address CodeRabbit review on RCA-11 PR #7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - metrics.py: fail-closed n_cases mismatch guard in regression_blocks_pivot - metrics.py: drop unreachable empty-union check in jaccard - run_rca11_extract_eval.py: sort transcriptions by page in loader - tests: use shared EXTRACTOR_VERSION constant instead of literal - SPEC.md: add language tag to §I fenced block (MD040) Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 1 + app/services/eval/metrics.py | 17 +++++++++++++++-- scripts/run_rca11_extract_eval.py | 7 ++++++- tests/test_kb_inbox.py | 4 ++-- tests/test_pdf_ingest_api.py | 3 ++- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index fa5f95c..5d97c7a 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,7 @@ mise.*.local.toml *.swp *.swo Thumbs.db +SPEC.md # ── Docker ──────────────────────────────────────────────────────────────────── postgres-data/ diff --git a/app/services/eval/metrics.py b/app/services/eval/metrics.py index a2e4cd7..69a7416 100644 --- a/app/services/eval/metrics.py +++ b/app/services/eval/metrics.py @@ -90,8 +90,6 @@ def jaccard(a: frozenset[str], b: frozenset[str]) -> float: if not a and not b: return 1.0 union = a | b - if not union: - return 1.0 return len(a & b) / len(union) @@ -133,6 +131,21 @@ def regression_blocks_pivot( mean_delta = candidate.mean_jaccard - baseline.mean_jaccard set_delta = candidate.set_equality_rate - baseline.set_equality_rate + # Fail closed on incomparable samples — deltas over different case counts + # don't mean anything, so never let a mismatch pass the gate. + if baseline.n_cases != candidate.n_cases: + return EvalReport( + baseline=baseline, + candidate=candidate, + mean_jaccard_delta=mean_delta, + set_equality_delta=set_delta, + blocks_pivot=True, + reason=( + f"n_cases mismatch: baseline={baseline.n_cases}, " + f"candidate={candidate.n_cases}" + ), + ) + reasons: list[str] = [] if mean_delta < -mean_jaccard_tolerance: reasons.append( diff --git a/scripts/run_rca11_extract_eval.py b/scripts/run_rca11_extract_eval.py index 9559f9d..2740d3a 100644 --- a/scripts/run_rca11_extract_eval.py +++ b/scripts/run_rca11_extract_eval.py @@ -57,7 +57,12 @@ async def _capture_transcriptions(pdf_path: Path, *, client: Any) -> list[dict[s def _load_transcriptions(path: Path) -> list[tuple[int, str]]: raw = json.loads(path.read_text()) - return [(int(r["page"]), str(r["text"])) for r in raw if str(r.get("text", "")).strip()] + rows = [ + (int(r["page"]), str(r["text"])) + for r in raw + if str(r.get("text", "")).strip() + ] + return sorted(rows, key=lambda x: x[0]) async def _extract_document( diff --git a/tests/test_kb_inbox.py b/tests/test_kb_inbox.py index cefdf5e..35aba8c 100644 --- a/tests/test_kb_inbox.py +++ b/tests/test_kb_inbox.py @@ -19,7 +19,7 @@ from app.models.atomic_fact import AtomicFact from app.models.outline import Course from app.services.kb.inbox import poll_inbox -from app.services.kb.pdf_ingest import RenderedPage +from app.services.kb.pdf_ingest import EXTRACTOR_VERSION, RenderedPage from tests._openai_mocks import make_client, make_completion @@ -69,7 +69,7 @@ async def test_poll_ingests_per_slug(db_session: AsyncSession, tmp_path: Path): facts = (await db_session.execute(select(AtomicFact))).scalars().all() assert len(facts) == 1 - assert facts[0].extractor_version == "pdf-vision-v2" + assert facts[0].extractor_version == EXTRACTOR_VERSION assert facts[0].node_id is None diff --git a/tests/test_pdf_ingest_api.py b/tests/test_pdf_ingest_api.py index 8ffd397..61f1a83 100644 --- a/tests/test_pdf_ingest_api.py +++ b/tests/test_pdf_ingest_api.py @@ -16,6 +16,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.models.outline import Course +from app.services.kb.pdf_ingest import EXTRACTOR_VERSION from tests._openai_mocks import make_client, make_completion _TOKEN = {"X-Coach-Token": "change_me_before_use"} @@ -64,7 +65,7 @@ async def test_pdf_ingest_happy_path(client: AsyncClient, db_session: AsyncSessi assert body["pages"] == 1 assert body["new_facts"] == 1 assert body["reused_pdf"] is False - assert body["extractor_version"] == "pdf-vision-v2" + assert body["extractor_version"] == EXTRACTOR_VERSION async def test_pdf_ingest_unknown_course_404(client: AsyncClient):