docs/audit/AUDIT-2026-06-15-full-repo.md

@@ -19,10 +19,10 @@ the repo.
 | **A** | Grounding guard (`wiki.py` `_parse_claims`/`_run_grounding_guard`, quarantine) — shared by `synthesis.py` | ✅ documented |
 | **B** | `sources/` (arxiv, openalex, semanticscholar) | ✅ documented |
 | **C** | `parsing/` (grobid, nougat, mathpix, figures, tex) | ✅ documented |
-| D | `synthesis.py` orchestration (find_connections/gaps/improvements, conjectures, write_leads) | ⏳ next |
-| E | `wiki.py` rest (retrieve, compile_concept, cross-refs, index/log, embed) | ☐ |
-| F | `embed.py`, `quality.py`, `models.py` | ☐ |
-| G | `cli.py` rest (ingest/wiki/synthesis/provenance commands), `config.py` rest | ☐ |
+| **D** | `synthesis.py` orchestration (find_connections/gaps/improvements, conjectures, write_leads) | ✅ documented |
+| **E** | `wiki.py` rest (retrieve, compile_concept, cross-refs, index/log, embed) | ✅ documented |
+| **F** | `embed.py`, `quality.py`, `models.py` | ✅ documented |
+| **G** | `cli.py` rest (ingest/wiki/synthesis/provenance commands), `config.py` rest | ✅ documented |
 
 ---
 
@@ -234,3 +234,255 @@ deep review yet; flagged in the ledger. Parsing hygiene is otherwise good.
 > **Request (a) — second audit pass — is now complete:** §8 of the loop-1 doc
 > (grounding guard ✓ Part A, `sources/` ✓ Part B, `parsing/` ✓ Part C) is closed.
 > Remaining whole-repo parts D–G are the broader sweep.
+
+---
+
+## Part D — `synthesis.py` orchestration
+
+The lead pipeline (connections → gaps → improvements → conjectures) reuses the
+Part-A grounding guard for the three grounded kinds and degrades gracefully on
+LLM/DB failure (`_safe_llm_generate`/`_list_paper_titles` swallow + log). The
+conjecture invariants are genuinely solid (status hard-set `"unverified"`,
+mandatory falsification path or the lead is dropped, written to `conjectures/`
+only). Two real defects and a doc contradiction:
+
+### C-11 — Lead-ID collision silently overwrites synthesis output · **HIGH**
+- **Evidence:** every stage restarts its counter at `seq = 1`
+  (`synthesis.py:341` connections, `:415` gaps, `:530` improvements,
+  `:634` conjectures) and ids are `L-0001, L-0002, …` (`_make_lead_id`). The CLI
+  concatenates the grounded kinds — `all_leads = connections + gaps + improvements`
+  (`cli.py:421`) — and `write_leads` routes **all** grounded kinds to the *same*
+  `grounded/<id>.md` (`synthesis.py:783-796`), writing in list order.
+- **Bug:** connection `L-0001`, gap `L-0001`, improvement `L-0001` all target
+  `grounded/L-0001.md`; later writes overwrite earlier → **improvements clobber
+  gaps clobber connections** at each number. Surviving files =
+  `max(#connections, #gaps, #improvements)`, **not** the sum. With 5/3/4 leads you
+  get 5 files, not 12; connections are lost first. Silent data loss of the
+  synthesis engine's core output, masked because "files do get written".
+- **Fix direction:** namespace ids by kind (`L-C-0001` / `L-G-0001` / `L-I-0001`)
+  or use a single shared counter across stages.
+
+### R-8 — Default-topic gap detection is a false-positive generator · **MED**
+- **Evidence:** `find_gaps` defaults `probe_topics` to paper **titles** when
+  `topics is None` (`synthesis.py:418-420`); a title query retrieves mostly its
+  *own* paper's chunks → `bibkeys_for_topic` ≈ 1, which is `< synthesis_gap_min_coverage`
+  (`:450-453`) → the topic is flagged as a gap and the LLM is asked to articulate
+  one.
+- **Impact:** nearly every ingested paper becomes a spurious "Gap: '<title>'
+  covered by only 1 bibkey(s)" candidate — the LLM invents a gap for material that
+  is actually well covered. The grounding guard catches some, but the default
+  signal is mostly noise. (User-supplied `--topics` avoid this.)
+
+### D-4 — `write_leads` vs `_update_index` docstrings contradict each other · **LOW**
+- **Evidence:** `write_leads` (`synthesis.py:762-765`) claims it "maintains an
+  **append-only** `INDEX.md`: a previously-recorded id is kept on subsequent runs",
+  but `_update_index` (`:802-803`) states — and implements — a "**full rebuild**,
+  not append-only" by globbing the on-disk dirs.
+- **Impact:** there is no append-only / id-retention logic; the index purely
+  reflects current disk state. The guarantee in the `write_leads` docstring is
+  false (and interacts with C-11: overwritten ids simply vanish from the index).
+
+**Part D net:** 1 HIGH (C-11, silent lead overwrite), 1 MED (R-8, gap noise),
+1 LOW (D-4, doc contradiction). The conjecture path is exemplary; the grounded
+path's persistence layer is where it breaks.
+
+---
+
+## Part E — `wiki.py` rest (compile, cross-refs, state, index/log)
+
+Good: `yaml.safe_load`, deterministic `_chunk_hash` (sorted by chunk id) driving a
+sound incremental-skip, append-only `log.md`, inline-code protection during
+cross-ref injection. The issues are math-corruption, a dead step, and a marking
+order bug.
+
+### R-9 — Cross-ref injection corrupts LaTeX math · **MED**
+- **Evidence:** `_inject_cross_refs` (`wiki.py:426`) protects only inline-code
+  spans (`_INLINE_CODE_RE`); it then runs whole-word, case-insensitive title/alias
+  → `[[slug]]` replacement over everything else (`:429-438`). **Math spans
+  (`$…$`, `$$…$$`, `\(…\)`) are not protected.**
+- **Impact:** the synthesis prompt explicitly asks for LaTeX `$ … $`
+  (`wiki.py:495`), so bodies contain math. A concept title/alias that appears
+  inside a formula (e.g. a concept named "Energy" inside `$E_{\text{Energy}}$`,
+  or a single-word alias) is rewritten to `[[slug]]`, corrupting the LaTeX. The
+  risk scales with how common the concept titles/aliases are as math tokens.
+
+### C-12 — `_try_embed_formulas` is a no-op that still costs a DB round-trip per concept · **LOW**
+- **Evidence:** `wiki.py:623-637` opens a connection, queries
+  `information_schema.tables` for `formulas`, and returns — the actual embed is
+  `"(Future: …)"`. Called unconditionally per concept (`compile_concept:609`).
+- **Impact:** step 4 of `compile_concept`'s docstring ("Embed formula chunks")
+  does nothing, and each concept compile pays an extra DB connection + query for
+  no effect. Placeholder masquerading as a feature.
+
+### C-13 — Ungrounded-claim marking can miss claims mutated by cross-ref injection · **LOW**
+- **Evidence:** order in `compile_concept` — claims parsed from `raw_output`
+  (`:602-603`), then `raw_output` is rewritten by `_inject_cross_refs` (`:606`),
+  then `_render_page_markdown` (`:612`) re-finds the *original* `claim.text` by
+  regex in the **injected** body (`:544-552`).
+- **Impact:** if an ungrounded claim's text contained a term that got replaced
+  with `[[slug]]`, the original text no longer matches → the `⚠` mark is silently
+  not applied. (Also note: wiki's marking lacks the `(?<!⚠ )` guard that
+  synthesis's `_mark_ungrounded` has — two divergent mark implementations.)
+
+### R-10 — Non-atomic compile-state write · **LOW**
+- **Evidence:** `_save_compile_state` (`wiki.py:475-477`) writes
+  `.compile-state.json` in place. A crash mid-write corrupts it; `_load`
+  then catches `JSONDecodeError` → `{}` → every concept recompiles. Graceful but
+  loses incrementality. Write to a temp file + `os.replace` for atomicity.
+
+### R-11 — `load_concepts` raises `KeyError` on malformed YAML · **LOW**
+- **Evidence:** `entry["slug"]` / `entry["title"]` (`wiki.py:135-136`) are
+  unguarded; a concepts.yaml entry missing a key crashes the whole compile with a
+  bare `KeyError` rather than a actionable message.
+
+> **Minor:** `compile_all` builds the index from `state.keys()` (`:745`), which
+> accumulates slugs across runs — a concept removed from `concepts.yaml` lingers
+> in `index.md`/state until manually pruned. Low.
+
+**Part E net:** 1 MED (R-9, math corruption), 4 LOW (C-12, C-13, R-10, R-11). The
+incremental-compile machinery is sound; the rendering/cross-ref layer has the rough
+edges. `OllamaClient.generate` (no retry, 120 s timeout, `raise` on error) feeds
+directly into **C-4** (its exception → empty page → published), so C-4's blast
+radius includes every transient Ollama hiccup.
+
+---
+
+## Part F — `embed.py`, `quality.py`, `models.py`
+
+The best-built corner of the repo. `embed.py` L2-normalises correctly (cosine ⇒
+dot product), guards empty input and zero-norm rows, and caches a process
+singleton. `quality.py` thresholds are all config-driven, `run_quality_pass` is
+CLI-wired (`cli.py:513-517`) and atomic (single commit). `models.py` dataclasses
+mirror the schema cleanly. Two dead/low-signal items and one corroboration:
+
+### C-14 — The sparse-embedding ("hybrid") path is dead code · **LOW**
+- **Evidence:** `Embedder.encode_sparse` / `encode` / `_coerce_sparse` are called
+  nowhere in `codex/` (grep for `encode_sparse` / `.encode(` finds only
+  `str.encode` in `wiki.py:455`). Only `encode_dense` is wired (ingest, search,
+  wiki). There is no sparse column in the schema.
+- **Impact:** the `embed.py` header and ADR-0002 advertise "**Hybrid dense +
+  sparse** embeddings via BGE-M3" as a headline, but the sparse half is
+  unreachable — the actual "hybrid" elsewhere is dense + Postgres FTS. Either wire
+  sparse retrieval or drop the capability + correct the docs.
+
+### R-12 — `classify_section` is low-signal as fed · **LOW**
+- **Evidence:** `classify_section` matches section headers in the **first 200
+  chars** (`quality.py:103-106`), but `tex.chunk_text` produces overlapping
+  *word-window* chunks that almost never begin at a header. So most chunks fall
+  through to `"body"` (or the DOI-density `"bibliography"` fallback).
+- **Impact:** the `chunks.section` column is mostly `"body"` — limited value for
+  any section-aware retrieval that depends on it. Not wrong, just weak signal
+  given the chunker.
+
+### Corroboration of C-7 (not a new finding)
+- `models.Paper.id` docstring (`models.py:20-21`) states the canonical id is "an
+  arXiv ID … **or a DOI (e.g. `10.1145/3592430`)**" — the **bare** form. Production
+  fills it from OpenAlex in **URL** form (C-7), so the code violates its own model
+  contract. This makes C-7 a documented-contract breach, not a judgement call.
+
+**Part F net:** 2 LOW (C-14, R-12) + a C-7 corroboration. These three modules are
+clean; no correctness defects.
+
+---
+
+## Part G — `cli.py` rest + `config.py`
+
+Most CLI commands are thin Typer wrappers delegating to already-audited modules
+(`ingest`, `search`, `discover`, `wiki`, `synthesis`, `provenance`, `quality`,
+`graph`). Settings are pydantic-validated with sensible bounds (`gt=0`, `0..1`).
+Findings are config-coupling and output hygiene.
+
+### C-15 — `embedding_dim` is a footgun decoupled from the hardcoded schema · **MED**
+- **Evidence:** `embedding_dim` is configurable (`config.py:67-74`, default 1024)
+  and `.env.example`/`schema.sql:7` invite other models (Qwen3 1024, **Jina v4
+  2048**). But `schema.sql` hardcodes `vector(1024)` on every embedding column
+  (`:25,38,102,119`), and `Embedder` derives the real output dim from the *model*,
+  using the `dim` setting only for empty-input zero-vectors (`embed.py:50,89`).
+- **Impact:** set `EMBEDDING_DIM=2048` (or pick a non-1024 model) and inserts
+  fail on a pgvector dimension mismatch — silently for empty abstracts
+  (`(1,2048)` zero-vector into `vector(1024)`), structurally for real vectors.
+  Nothing couples the knob to the DDL; the setting reads as supported but is not.
+
+### R-13 — `export_bib` does no BibTeX escaping and forces `@article` · **LOW**
+- **Evidence:** `provenance.py:174-181` interpolates `title`/`author`/`year` raw
+  into `@article{…}`; no escaping/brace-balancing of BibTeX specials
+  (`{ } \ % # $ _ &`).
+- **Impact:** a title with an unbalanced brace or a stray special char produces a
+  malformed `.bib` entry that breaks Doxygen/BibTeX. Also every paper is emitted
+  as `@article` although the corpus includes theses/books/chapters
+  (`ingest_all.sh`) — bibliographically wrong (functional for `@cite` keys though).
+
+### S-4 — Secrets stored as plain `str`, not `SecretStr` · **LOW**
+- **Evidence:** `database_url`, `mathpix_app_key`, `mcp_auth_token`
+  (`config.py:29,148,249`) are plain `str`. Any `repr(Settings())` or debug log of
+  the settings object prints the DB password / API key / bearer token in clear.
+- **Impact:** log/exception-dump leakage surface (complements S-1). Use
+  `pydantic.SecretStr` for credential fields.
+
+### R-14 — `mcp_transport` enum not enforced · **LOW**
+- **Evidence:** `config.py:229-236` documents "stdio or http" but no validator;
+  `main()` (`mcp_server.py:323`) treats anything non-`http` as stdio. A typo
+  (`htttp`) silently starts stdio instead of failing loudly.
+
+### D-5 — Inconsistent `validation_alias` on `nougat_url` only · **LOW**
+- **Evidence:** `nougat_url` adds `AliasChoices("NOUGAT_URL", "nougat_url")`
+  (`config.py:47`) that no other field has — redundant under
+  `case_sensitive=False`, and the inconsistency invites confusion.
+
+> **Corroboration of C-7:** `cli.py:45` ingest does **no** id normalization and
+> echoes `result.paper_id` (the URL-form id) straight back — no normalization
+> layer exists at any tier (source → ingest → CLI).
+
+**Part G net:** 1 MED (C-15, dim/schema footgun), 4 LOW (R-13, S-4, R-14, D-5).
+CLI/config are otherwise solid and well-documented.
+
+---
+
+# Executive Summary — whole-repo audit complete (loop-1 + Parts A–G)
+
+**Tally: 45 findings — 8 HIGH, 11 MED, 26 LOW/INFO.** Test suite 330 green,
+`ruff`/`mypy` clean. The codebase is well-structured and idiomatic; the defects
+cluster into five root themes, not random scatter.
+
+### The 8 HIGH findings
+| ID | Theme | One-liner |
+|----|-------|-----------|
+| S-1 | secrets | live DB password in un-ignored `.env.jetson-ingest` (mitigated this session) |
+| M-1 | migration | `paper_identifiers` has no migration path; `schema.sql` not re-applyable, `apply_schema` never called |
+| C-1 | identity | `discover.py` keeps the ID-mismatch bug F-15 fixed in `graph.py` (over-reports ingested papers as leads) |
+| C-7 | identity | `papers.id` stored in OpenAlex **URL** form, violating the bare-ID contract (breaks lookups + cross-citation match) |
+| C-4 | silent failure | zero-citation/LLM-outage wiki pages **bypass quarantine** and publish as "compiled" |
+| C-11 | silent data loss | synthesis leads collide on `L-000N` across stages → improvements overwrite gaps overwrite connections |
+| D-1 | doc integrity | ADR-F15 GO rests on spike numbers measured **before** the shipped resolution fix |
+| T-1 | verification | the F-15 resolution SQL (the substance of the last 2 commits) has **zero** real-DB coverage |
+
+### Five root-cause themes
+1. **Un-normalized identity (the dominant theme).** C-1, C-7, C-10, M-1, T-3,
+   and most of the F-15 ID dance are one problem: a paper's identity exists in
+   many un-reconciled forms (bare vs URL · DOI vs arXiv vs OpenAlex-W vs S2-hash
+   vs PDF-filename) with **no canonical normalization layer**. One normalization
+   boundary at ingest would retire ~5 HIGH/MED findings at once.
+2. **"Green but unverified."** T-1/T-2/T-3 — the most error-prone code (resolution
+   SQL, migrations, id-mapping) is mocked or tested with unrepresentative fixtures,
+   so real bugs (C-4, C-7, C-11) sit behind 330 passing tests.
+3. **Silent failure / data loss under degradation.** C-4 (publish empty), C-11
+   (overwrite leads), R-1 (over-quarantine) corrupt *outputs* instead of erroring
+   — the worst failure mode for a research tool whose value is trust.
+4. **Overstated guarantees.** D-1 (stale ADR), D-2/D-4 (doc↔code drift), C-5
+   ("zero-fact-leak" ≫ a last-sentence substring check), C-14 ("hybrid" sparse is
+   dead) — the docs/commit-language claim more than the code delivers.
+5. **Latent rich-path debt.** C-10, R-6, R-7 are real but dormant because the live
+   corpus is `.txt`; they detonate on the first `.pdf --rich` ingest.
+
+### Suggested remediation sequencing (themes, not a plan — planning deferred)
+- **First:** S-1 (rotate credential — secret already protected), M-1 (migration
+  path), C-7+C-1 (one identity-normalization layer), T-1 (real-DB test) — these
+  unblock trust in the live corpus.
+- **Then:** C-4 + C-11 (stop silent output corruption), D-1 (re-validate ADR-F15).
+- **Then:** MED batch (R-1, R-8, R-9, C-15, C-10) and the LOW polish.
+
+### Coverage honesty
+Every `codex/` module was read. Not deep-reviewed: `nougat.py` (54-line `.pdf`
+wrapper, skimmed), and the exact runtime DB state (URL-form ids in C-7, and the
+`paper_identifiers` existence in M-1) is **inferred from code** and flagged
+"confirm on the Jetson DB" — I could not query the live database from here.