Populate Toulmin qualifier/warrant/rebuts from the FOOTNOTES_JSON self-report by CServinL · Pull Request #105 · CServinL/prisma

CServinL · 2026-09-11T06:44:53Z

Toulmin argumentation: populate qualifier/warrant/rebuts

Phase B of the knowledge-graph-tools grand plan (Phase A: #104). The Toulmin schema —
Qualifier enum, WarrantNode, and qualifier/warrant/rebuts on
CitedClaimNode/InferenceNode (#78) — shipped with nothing populating it: migrations,
committed JSON schemas, session_graph.py's WARRANTS/REBUTS edges, and the UI's
qualifier badge / warrant block / rebuts jump-link were all already in place, waiting.
This wires the population, through the same typed-block + deterministic-validation path
sources/relation already use in the FOOTNOTES_JSON self-report — no schema or
migration change.

What changed

_RawFootnote (chat_agent.py) gains qualifier/warrant/rebuts as optional keys,
validated on parse exactly like relation — a malformed value fails that entry, not a
silent degrade to "field omitted."
rebuts is same-turn only: the model's self-report has no handle on a claim besides
its own answer's [^N] index, so _resolve_rebuts translates that index to the target
claim's real id once every claim in the turn is built. An index with no match, or
equal to the claim's own, drops the whole claim.
ChatAgent._warrant_resolves hard-validates warrant.backing exactly like
_sources_resolve validates sources — an unresolvable slug drops the claim. The two
failure modes log distinct messages.
system_prompt_footnote_section() declares the three keys as an optional addition to
the existing FOOTNOTES_JSON block.
Frontend's rebuts jump-link was dead code assuming claim.rebuts was a per-turn index;
it's actually another claim's node id — fixed to resolve it via an id → index map
built once per turn (not a .find() inside the render loop).

Review history

Three local review passes before opening this PR: a blank/whitespace-only warrant.text
slipping past validation, a diagnosability gap in a combined drop-warning, a direct
attribute mutation inconsistent with this file's model_copy(update=...) convention, an
unbounded warrant.backing list, and a redundant validation constraint left over from
fixing the first of these. All fixed with regression tests confirmed to fail on the
pre-fix code.

Verification

Full suite (1304 tests) green, svelte-check 0 errors, npm run build clean, diagrams
regenerated with no diff.

…f-report Phase B of the KG-tools grand plan. The schema (Qualifier enum, WarrantNode, qualifier/warrant/rebuts on CitedClaimNode/InferenceNode), migrations, committed JSON schemas, session-graph WARRANTS/REBUTS edges, and UI rendering all shipped already -- every one documented "schema support only, nothing populates these yet." This wires the population, through the same typed-block + deterministic-validation path sources/relation already use, not prompt-behaviour coaching: - _RawFootnote gains the three optional keys, validated on parse exactly like relation -- an unknown qualifier or an empty warrant.text fails the whole entry. - rebuts is same-turn only: the model's self-report has no handle on a claim besides its own answer's [^N] index, so _resolve_rebuts translates that index to the target claim's real id after every claim in the turn is built. An index with no match, or equal to the claim's own, drops the whole claim -- the same "an unsatisfied part of the block rejects the entry" rule sources/relation already enforce. - ChatAgent._warrant_resolves validates warrant.backing exactly like _sources_resolve validates sources. - system_prompt_footnote_section() declares the three keys as an optional addition to the existing FOOTNOTES_JSON block. - Frontend's rebuts jump-link was dead code assuming claim.rebuts was an index; fixed to resolve the id it actually receives to the target's index. 11 new backend tests + 1 prompt test, all confirmed red on pre-change code. Full suite 1302 passed, svelte-check 0 errors, npm run build clean.

_RawWarrant.text's min_length=1 counted raw characters, so " " passed where "" was correctly rejected -- a warrant with nothing to say isn't a warrant regardless of whether "nothing" is zero characters or all whitespace. Verified red on pre-fix code.

… diff - Split the single "dropped claim(s) citing an unresolvable slug" warning into two: a bad sources slug and a bad warrant.backing slug point at different parts of the self-report to fix, and the combined message couldn't tell them apart. - _resolve_rebuts now uses claim.model_copy(update=...) instead of direct attribute assignment, matching the immutable-update convention _verify_claim already uses in this same file. - The UI's rebuts jump-link did an O(n) msg.claims.find() per claim inside the claims {#each} -- O(n^2) per turn. Replaced with a single id->index Map built once per turn. - Also capped _RawWarrant.backing at 20 entries: each entry costs a ChatAgent._warrant_resolves() vault lookup, and unlike sources (pre- existing, out of this diff), this is new code that shouldn't repeat an unbounded-list pattern from scratch. 4 new/changed tests (oversized backing, the two now-distinct log messages via caplog), all confirmed red on pre-fix code. Also parametrized the two identical-shape blank-warrant-text tests. Full suite 1304 passed, svelte-check 0 errors, npm run build clean.

_reject_blank's not v.strip() check already rejects "" (and now ""'s superset, whitespace-only text) -- min_length=1 caught nothing the validator didn't already cover, just dead weight sitting next to it.

Copilot

🟡 Changes recommended

Rebuttals can resolve to removed or ambiguous claims, producing dangling or incorrect references.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Toulmin qualifier, warrant, and rebuttal population to chat claims through FOOTNOTES_JSON.

Changes:

Parses and validates optional Toulmin metadata.
Resolves backing slugs and same-turn rebuttal references.
Updates UI rendering, tests, and documentation.

File summaries

File	Description
`prisma/agents/chat_agent.py`	Parses, validates, and resolves Toulmin fields.
`prisma/services/chat_tools.py`	Documents fields in the model prompt.
`ui/src/routes/+page.svelte`	Resolves rebuttal IDs for jump links.
`tests/unit/agents/test_chat_agent.py`	Tests parsing and validation behavior.
`tests/unit/services/test_chat_tools.py`	Tests prompt coverage.
`docs/concepts/claim.md`	Documents populated claim fields.
`docs/concepts/chat-session-graph.md`	Documents graph integration and validation.

Review details

Suppressed comments (1)

prisma/agents/chat_agent.py:247

Duplicate footnote indices make this lookup ambiguous: the dict silently keeps the last claim for an index, while the UI's index-based DOM lookup lands on the first matching element. A rebuts link can therefore name one claim ID but jump to a different claim. Detect duplicate indices and treat references to them as invalid (or reject the duplicate entries) instead of selecting one implicitly.

        target = by_index.get(rebuts_idx)

Files reviewed: 7/7 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

_resolve_rebuts resolves a claim's rebuts index against by_index, a snapshot of every parsed claim taken before any drops happen -- so claim B can resolve rebuts to claim A's real id while A is still a valid same-turn index, then A gets dropped anyway (its own invalid rebuts within the same function, or later in respond() for an unresolvable sources/warrant. backing slug B has no bearing on). B would then survive with a rebuts id pointing at a claim no longer in the turn -- session_graph.py's REBUTS edge would silently create a phantom, data-less node for it (NetworkX auto-creates any edge endpoint that isn't already a node). New _prune_dangling_rebuts: fixed-point-drops a claim whose rebuts id doesn't match a currently-surviving claim, repeated until a full pass removes nothing (so a rebuttal chain cascades correctly, not just one hop). Called from both _resolve_rebuts (catches the intra-turn case) and the end of ChatAgent.respond()'s filter (catches the cross-function case, after sources/warrant.backing resolution drops claims _resolve_rebuts has no visibility into). 2 new regression tests, both confirmed red on pre-fix code. Full suite 1306 passed.

Copilot

🔵 Needs a closer look

Rebuttal validation currently permits unintended coercions and resolves duplicate indices ambiguously.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

prisma/agents/chat_agent.py:148

rebuts is documented as accepting only an integer or the explicit "N"/"[^N]" forms, but Pydantic's non-strict int also coerces values such as true (to 1) and 1.0. A malformed self-report can therefore create a valid-looking REBUTS edge instead of dropping the entry as promised. Make the post-validator integer strict; the mode="before" validator will still convert the supported string forms first.

This issue also appears on line 248 of the same file.

prisma/agents/chat_agent.py:248

This lookup silently overwrites duplicate footnote indices. With two claims at index 1 and a third claim rebutting 1, the graph edge targets whichever duplicate appeared last, while the UI's duplicate chat-turn-…-claim-1 IDs can jump to the first one. Because rebuts now relies on index uniqueness, reject duplicate-index entries (and references to that ambiguous index) instead of resolving them order-dependently.

    by_index = {c.index: c for c, _ in built}

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

…lot) Two more findings from the same review round: - rebuts was plain `int`, which Pydantic's lax mode silently coerces a bool ("rebuts": true -> 1) or a whole-number float ("rebuts": 1.0 -> 1) into -- neither is the index the model actually meant, and either would build a real REBUTS edge from what's actually a malformed self-report instead of dropping the entry. Switched to StrictInt; the existing mode="before" validator still converts the "[^N]"/"N" string forms first, since a before-validator hands StrictInt an already-real int for those. - Two entries sharing the same `index` made by_index's {index: claim} mapping pick one arbitrarily (whichever came last), while the UI's id="chat-turn-N-claim-{index}" DOM anchor collides the same way (two <li> elements with the same id). Because rebuts now relies on index uniqueness for a meaningful jump-link, _resolve_rebuts now drops every claim sharing a duplicate index up front, before by_index is even built -- the whole group is untrustworthy, not just whichever rebuts happens to target it. 5 new/changed tests, all confirmed red on pre-fix code (the non-integer tests specifically avoid the self-reference/no-such-index paths so they actually exercise StrictInt, not an unrelated existing check). Full suite 1310 passed.

CServinL · 2026-09-11T07:19:42Z

Addressed the two findings from the 07:11 UTC review (posted as suppressed comments, no inline thread to reply to directly) in 4156e89:

rebuts accepting non-integer values (chat_agent.py:148) — rebuts was plain int, so Pydantic's lax mode silently coerced "rebuts": true → 1 and "rebuts": 1.0 → 1, building a real REBUTS edge from a malformed self-report instead of dropping it. Switched to StrictInt; the existing mode="before" validator still converts the "[^N]"/"N" string forms first, since it hands StrictInt an already-real int for those.
Duplicate footnote indices (chat_agent.py:248) — two entries sharing the same index made by_index's mapping pick one arbitrarily (whichever came last), while the UI's id="chat-turn-N-claim-{index}" DOM anchor collides the same way. _resolve_rebuts now drops every claim sharing a duplicate index up front, before by_index is built — the whole group is untrustworthy, not just whichever rebuts targets it.

5 new/changed regression tests, all confirmed red on pre-fix code. Full suite 1310 passed.

The rebuts fix's review comment cross-referenced this same line -- the identical plain-`int` gap existed on `_RawFootnote.index` too, and it's the more consequential half: `index` is the primary key [^N] marker matching, duplicate-index detection, and rebuts resolution all key off, so a silently-coerced `"index": true`/`1.0` corrupts more than just one field's value. No mode="before" validator needed here (unlike rebuts, the model is never taught a "[^N]" string form for its own index). 2 new tests, confirmed red on pre-fix code. Full suite 1312 passed.

CServinL · 2026-09-11T07:24:00Z

Good catch — no, I'd only fixed rebuts. The cross-reference to line 248 in that finding was pointing at _RawFootnote.index, which had the identical plain-int gap ("index": true/1.0 silently coerced to 1) — and it's the more consequential of the two, since index is the primary key [^N] marker matching, duplicate-index detection, and rebuts resolution all key off. Fixed in b839bc4 (StrictInt, same as rebuts), with 2 new tests confirmed red on pre-fix code. Full suite 1312 passed.

Copilot

🟡 Changes recommended

Claim indices remain insufficiently validated, and inference warrants are not coherently described to the model.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Balanced

… too Two more findings from the same review round: - StrictInt alone still let 0/negative values through for _RawFootnote. index -- strictly-typed, but not a real 1-based [^N] marker. Added Field(gt=0). - system_prompt_footnote_section()'s warrant description defined it only as "why the sources support this claim," but InferenceNode (ai- inference, always empty sources) is schema-supported for warrant too -- the model had no coherent instruction for that case at all. Reworded to cover both: a sourced claim's warrant explains why the sources support it; an ai-inference warrant explains the reasoning process itself, with backing left empty since there's nothing to cite. 6 new/changed tests, all confirmed red on pre-fix code. Full suite 1315 passed.

Copilot

🟡 Changes recommended

Rebuttal navigation omits schema-valid cross-turn targets, and the prompt does not expose the backing-list limit.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 7/7 changed files
Comments generated: 3
Review effort level: Balanced

Two of the three new review findings: - _MAX_WARRANT_BACKING existed only on the parser side -- the model was never told the cap exists, so a self-report that (correctly, per the instruction it *was* given) cited 21 real sources got its entire otherwise-valid claim silently dropped. Moved the constant to chat_tools.py (public, MAX_WARRANT_BACKING) -- the producer's home, alongside FOOTNOTES_LINE_RE/TOOL_CALL_RE which chat_agent.py already imports from there, so the reverse import direction (chat_tools importing from chat_agent) isn't needed and wouldn't have worked anyway (chat_agent already imports chat_tools -- that would be circular). system_prompt_footnote_section() now states the number. - The rebuts jump-link's id->index map was built from one turn's own msg.claims -- but CitedClaimNode.rebuts/InferenceNode.rebuts is an unrestricted claim id at the schema level, and session_graph.py's REBUTS edge already supports a cross-turn target (test_session_graph.py has one); only the new model self-report resolver is same-turn-limited. The renderer inherited that producer's narrowness for free, silently hiding any cross-turn rebuts with no error, just a missing button. Replaced with a chat-wide id->{turn, index} map ($derived over activeChat.messages). New test for the prompt side, confirmed red on pre-fix wording. svelte-check 0 errors, npm run build clean. Full suite 1316 passed.

The Toulmin fields' concept docs (claim.md, chat-session-graph.md) already said "populated" -- vault_models.py's own docstrings on WarrantNode and CitedClaimNode still said "schema support only, nothing populates these yet," directly contradicting the concept docs a reader might not even check. Updated both. Swept the whole repo for the same "schema only / nothing populates this yet" phrase (the user's ask: same pattern, other places) and found it had drifted stale independently of this PR, at several points where a feature shipped but the status label describing it never got revisited: - vault_models.py's TurnNode.media/attachments comment blanket-labeled both "schema support only" -- attachments *is* populated (app.py wires ChatRequest.attachments/attached_slugs through); only media/PRODUCES (the assistant-output direction) genuinely has no generator. - docs/concepts/chat.md: `thoughts` and the Toulin qualifier/warrant/ rebuts row both still said "nothing populates this yet" -- thoughts shipped 2026-08-18 (the THINK: tool), Toulmin as of this PR. - docs/concepts/chat-session-graph.md's own node/edge table: ThinkingNode, InlineMediaNode/AssetMediaNode, and the WARRANTS/REBUTS/PRODUCES/ ATTACHES edge rows all still said "v3, on branch (unmerged)" -- for a branch merged as PR #78 weeks earlier. This table contradicted the same doc's own dated Status section below it. - docs/wiki/roadmap.md and docs/wiki/data-models.md: both had a top-level "not yet merged" for the entire Toulmin/media/attachments/reasoning feature set. Regenerated the 5 affected committed JSON schemas (docstring-only diffs, confirmed by `test_committed_schemas_match_the_live_models`). Saved the defect classes from this whole PR (9 Copilot findings across 6 rounds) as docs/chat-claims-review-checklist.md, referenced from .claude/CLAUDE.md -- same pattern as docs/kg-retrieval-review-checklist.md. Full suite 1316 passed.

Copilot

🟡 Changes recommended

Malformed rebuttal strings can be accepted, and some valid cross-turn rebuttal links still navigate to no target.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

prisma/agents/chat_agent.py:177

lstrip/rstrip do not validate the delimiters; they remove arbitrary repetitions independently, so malformed values such as [1], ^^1, or [[^1]] are normalized to 1 and can create a real REBUTS edge. Match only the two documented string forms (N and [^N]) before converting, leaving everything else for StrictInt to reject.
docs/concepts/chat-session-graph.md:222
The model defines five media kinds (svg, latex, drawio, jpg, and pdf), and the upload section below also says “all five kinds.” Calling this four makes the lifecycle status inaccurate.

ui/src/routes/+page.svelte:127

This API-facing type comment incorrectly limits rebuts to the same turn. InferenceNode.rebuts has the same unrestricted claim-ID contract as CitedClaimNode.rebuts, which is why the renderer now builds a chat-wide lookup.

    rebuts: string | null;  // another claim's `id` in this same turn, not its index

Files reviewed: 18/18 changed files
Comments generated: 3
Review effort level: Balanced

.claude/CLAUDE.md and the two review checklists (kg-retrieval, chat-claims) are Claude Code's own working notes/instructions -- agent-facing operational memory, not user-facing project documentation. They don't belong committed to a public repo: - .gitignore: added .claude/ - .claude/CLAUDE.md: untracked (git rm --cached), kept locally; its two checklist references updated to their new .claude/-local paths - docs/kg-retrieval-review-checklist.md -> .claude/kg-retrieval-review-checklist.md - docs/chat-claims-review-checklist.md -> .claude/chat-claims-review-checklist.md Also updated the global ~/.claude/CLAUDE.md instruction that had suggested docs/<area>-review-checklist.md as the destination for this class of content -- it now says .claude/<area>-review-checklist.md, gitignored, so this doesn't recur on another project. No code change; full suite still 1316 passed.

…docs - The rebuts jump-link could point at a claim whose turn is wholeTurnInference (the References block -- and its claim anchor -- is suppressed for those turns). The button rendered (rebutsTargetById still resolves it) but clicked into nothing. New scrollToClaimOrTurn() falls back to the turn container itself when the specific claim anchor doesn't exist, checked at click time rather than duplicating isWholeTurnInference's logic into the map. - WarrantNode's docstring (and claim.md's field table) still defined a warrant purely as "why sources support this claim" -- but _claim_from_raw populates warrants on InferenceNode too (no sources at all), and the prompt already describes that case as the model's own reasoning process. Reworded both to cover both cases; regenerated the 5 affected committed schemas. - The CitedClaimOut/InferenceClaimOut TS comments still said rebuts was "in this same turn" -- stale the moment the chat-wide cross-turn lookup was added. Reworded at both sites. svelte-check 0 errors, npm run build clean. Full suite 1316 passed (no UI test framework exists in this repo -- verification bar for .svelte changes is svelte-check + build + manual reasoning, same as prior rounds).

Copilot

🟡 Changes recommended

Some inference metadata is discarded or hidden, and an unrelated review checklist is removed.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/kg-retrieval-review-checklist.md:1

This removes the repository's only KG retrieval review checklist, although the PR is scoped to Toulmin claim population and provides no replacement. The deleted document captures still-applicable correctness constraints for provenance, locking, bounded reads, and slug safety; losing it is unrelated and makes future KG changes harder to review safely. Restore the file here or move its removal to a separately justified change.

Files reviewed: 18/19 changed files
Comments generated: 2
Review effort level: Balanced

Two more findings from the same review round: - respond()'s no-grounding override collapses the model's self-report to one bare InferenceNode(index=1, claim_text=content) -- discarding any qualifier/warrant the model had put on an ai-inference claim, exactly the case system_prompt_footnote_section() now explicitly describes (own reasoning, no sources). Now carries qualifier/warrant over from the original self-report when there's exactly one claim to take them from -- unambiguous only in that case, so 2+ claims still collapse to a bare InferenceNode as before. rebuts is never carried over: it'd reference an id about to be discarded along with every other claim here, the same dangling-reference shape _prune_dangling_rebuts exists to prevent. - A whole-turn-inference turn (isWholeTurnInference) renders no claim row at all (the References block is suppressed for it, by design -- the whole turn already reads as one uncited claim). That meant any qualifier/warrant/rebuts on that turn's sole claim -- newly reachable after the fix above -- had nowhere to display. Added a compact Toulmin section to the inference card itself, reusing the exact same qualifier-badge/warrant-block/rebuts-button markup and CSS classes the claim-list rendering already uses. 2 new backend tests (one confirming preservation, one confirming the ambiguous 2+-claims case still drops to None), confirmed red/green. svelte-check 0 errors, npm run build clean. Full suite 1318 passed.

Copilot

🟡 Changes recommended

Inference handling can retain invalid source-backed warrant metadata, and the PR also removes an unrelated repository review checklist.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

docs/kg-retrieval-review-checklist.md:1

This PR is scoped to populating Toulmin fields, but it also removes the repository's entire KG retrieval review checklist, and there is no replacement checklist in the current tree. That drops the documented safety constraints for the Phase A retrieval surfaces without supporting this change; restore it or move this deletion to a separately explained change.

prisma/agents/chat_agent.py:617

This preserves Toulmin metadata from any sole claim, including a CitedClaimNode. When an empty grounding result forces that claim into an inference, its warrant may still explain the discarded sources (or expose their backing), so the UI labels source-derived rationale as “not from your vault.” Preserve metadata only when the sole self-report was already an inference.

                    qualifier = claims[0].qualifier if len(claims) == 1 else None
                    warrant = claims[0].warrant if len(claims) == 1 else None

Files reviewed: 18/19 changed files
Comments generated: 2
Review effort level: Balanced

* Move investigation-log docs out of the public repo; genericize 2 more real-paper mentions Same class as the previous .claude/ privacy cleanup (PR #105), found while checking docs/kg-dead-letter-triage-2026-07-07.md at cservinl's flag: these are one-off engineering investigation/benchmark logs, not standing project documentation, and several reveal real vault paper filenames (the user's private research reading list) or personal machine specs incidentally, as test subjects/context for the investigation: - docs/kg-dead-letter-triage-2026-07-07.md -> .claude/ (dozens of real vault paper filenames, the user's thesis folder structure, username) - docs/kg-extraction-context-length.md -> .claude/ (real vault paper as the test subject) - docs/qwen3-family-evaluation.md -> .claude/ (personal GPU/hardware specs) - docs/llamacpp-vulkan-home-server-vs-desktop-client-benchmark.md -> .claude/ (home server + desktop client hardware specs) - docs/nuextract-2.0-kg-extraction-evaluation.md -> .claude/ (personal hardware specs; uses a public paper as its test content, but still an investigation log, not standing docs) - docs/ollama-concurrency.md -> .claude/ (personal GPU/hardware specs) Updated every cross-reference across TODO.md, config.example.toml, 3 ADRs, docs/wiki/configuration.md, the compute-pool-contention diagram source, the openrouter-free-models test harness, supervisor.py, knowledge_graph_ service.py, config.py, and their tests -- all comment/prose references, nothing load-bearing at runtime. Also genericized two more real-paper-filename mentions found in the same sweep, in docs that correctly stay public (ADR-013, TODO.md) -- the underlying architectural point (a paper too large for one extraction chunk) doesn't need the specific filename. .gitignore: added .claude/ on this branch too (it was cut before PR #105 merged that same line) -- caught a real near-miss while staging: `git add -A -- .claude/` briefly staged an unrelated stray full-repo worktree snapshot sitting under .claude/worktrees/ before this line existed on this branch; reset and re-staged explicit paths only. Full suite 1290 passed (this branch's baseline, off main). * Move investigation logs into docs/logs/, separate from product documentation Genericized (hardware specs, real-paper filenames -> academic citations) one-off engineering investigation/benchmark logs still belong in the public repo -- they're cited as the evidence base for real, shipped config decisions (token_budget, model_affinity, max_concurrent) -- but mixed in at docs/'s top level they read as product documentation, which they aren't. Same treatment for docs/tests/openrouter-free-models/: also a technical log (a benchmark run + its results), not a test suite (never in pytest's testpaths). - docs/kg-dead-letter-triage-2026-07-07.md -> docs/logs/ - docs/kg-extraction-context-length.md -> docs/logs/ - docs/qwen3-family-evaluation.md -> docs/logs/ - docs/llamacpp-vulkan-home-server-vs-desktop-client-benchmark.md -> docs/logs/ - docs/nuextract-2.0-kg-extraction-evaluation.md -> docs/logs/ - docs/ollama-concurrency.md -> docs/logs/ - docs/tests/openrouter-free-models/ -> docs/logs/openrouter-free-models/ (docs/tests/ is now gone entirely) Updated every cross-reference across TODO.md, config.example.toml, 3 ADRs, docs/wiki/configuration.md, the compute-pool-contention diagram source, supervisor.py, knowledge_graph_service.py, config.py, their tests, and the moved files' own internal cross-references to each other. Full suite 1290 passed.

…parse An ai-inference claim has no document behind it, so a non-empty warrant.backing on one is a self-contradiction. _RawFootnote now rejects that combo at parse time, and the no-grounding override -- which can collapse a CitedClaimNode (legitimately backed) into an InferenceNode -- strips the warrant instead of carrying the inconsistency across. Also fixes the inference warrant tooltip, which described grounds an inference by definition doesn't have. Copilot review, PR #105.

Copilot

🟢 Approval recommended

The implementation, UI behavior, tests, schemas, and documentation are consistent, with previously identified edge cases addressed.

Review details

Files reviewed: 18/19 changed files
Comments generated: 0 new
Review effort level: Balanced

Self-audit against the review checklist before the next Copilot pass, not after: the fix landed in chat_agent.py (1f50df5) but the concept docs' own field reference and validation-rules sections didn't mention the new rejection, matching the exact staleness pattern the checklist's own item #6 warns against.

…h-relevance Phase B, increments 2 and 3 of 4 (Toulmin shipped in #105; cross-chat entity linking deferred to its own session -- see the plan discussion). suggest_questions: same 4-layer pattern as god_nodes/surprising_connections (kg_queries.suggest_questions -> KnowledgeGraphService cache -> kg_app.py route + client -> chat_tools ToolSpec/handler). Phrases a grounded question per RelatesTo edge, deduped by entity pair, capped at one question per source_file before filling remaining slots. Stream triage: a lightweight text-overlap score (KnowledgeGraphService. graph_relevance, backed by a cached distinct_entity_labels() scan) against already-extracted KG entity labels -- no stream/Zotero content is ever indexed into Kùzu, keeping docs/concepts/stream.md's stated boundary intact. New GET /zotero/items/relevance endpoint scores and sorts the existing item listing; a new UI toggle in the Zotero browser panel calls it and renders a small match-count badge. Both the new /graph_relevance route (directly reachable on the kg worker) and its zotero_routes.py caller share one bound (kg_queries. GRAPH_RELEVANCE_MAX_TEXTS/_MAX_TEXT_LENGTH) rather than two literals that would drift. TODO.md's now-fully-shipped tool list corrected; docs/concepts/ zotero-item.md documents the new endpoint and its relationship to stream.md's KG-pollution boundary.

1610 -> 109 lines. TODO.md had become a chronological engineering journal spanning months -- resolved/historical narrative mixed in with genuinely open items, most checkboxes already stale by the time this session started (several already fixed by PRs #104/#105/#106, confirmed against the actual codebase rather than trusted at face value). Kept only what's still genuinely pending, verified against current code for each ambiguous item (chat-tier label, deletion cascade, graphify migration, streaming, container build, chart wiring, etc.) rather than assuming the checkbox state was accurate. Also dropped personal/private details incidental to the historical narrative (nothing left that belongs in a public repo's backlog). Cross-references to now-removed TODO.md sections fixed where they made an active, findable claim (config_reload.py, its test, ADR-004, ADR-008, ADR-015's header) -- either redirected to the code/ADR that's the real durable home, or the essential point inlined directly since the pointer was the only thing carrying it. Git history has the full removed content if any of it is ever needed again; docs/logs/ remains the home for investigation-log-shaped writeups, which is what the user asked TODO.md to stop doing.

…am-triage-by-graph-relevance (#106) * Rename docs/ontologia.md to docs/ontology.md English-only in a public repo (the file's content was always English -- just the filename was Spanish). Updated every reference across concept docs, TODO.md, ADR-017, .claude/CLAUDE.md, and vault_models.py's CitedClaimNode docstring; regenerated the 3 committed JSON schemas that embed that docstring. Full suite 1290 passed (this branch's baseline, off main). * Fix TODO.md staleness: mark KG chat tools shipped by PR #104 Per-section extraction, god_nodes, surprising_connections, expand_node, and the get_full_text/read_source bounded-excerpt case were all still marked '[ ]'/'not built yet' despite shipping in PR #104. Only suggest_questions and the consultation-sub-agent redesign remain genuinely unbuilt -- narrowed the framing to just those. * Build suggest_questions chat tool + lightweight stream-triage-by-graph-relevance Phase B, increments 2 and 3 of 4 (Toulmin shipped in #105; cross-chat entity linking deferred to its own session -- see the plan discussion). suggest_questions: same 4-layer pattern as god_nodes/surprising_connections (kg_queries.suggest_questions -> KnowledgeGraphService cache -> kg_app.py route + client -> chat_tools ToolSpec/handler). Phrases a grounded question per RelatesTo edge, deduped by entity pair, capped at one question per source_file before filling remaining slots. Stream triage: a lightweight text-overlap score (KnowledgeGraphService. graph_relevance, backed by a cached distinct_entity_labels() scan) against already-extracted KG entity labels -- no stream/Zotero content is ever indexed into Kùzu, keeping docs/concepts/stream.md's stated boundary intact. New GET /zotero/items/relevance endpoint scores and sorts the existing item listing; a new UI toggle in the Zotero browser panel calls it and renders a small match-count badge. Both the new /graph_relevance route (directly reachable on the kg worker) and its zotero_routes.py caller share one bound (kg_queries. GRAPH_RELEVANCE_MAX_TEXTS/_MAX_TEXT_LENGTH) rather than two literals that would drift. TODO.md's now-fully-shipped tool list corrected; docs/concepts/ zotero-item.md documents the new endpoint and its relationship to stream.md's KG-pollution boundary. * Fix 3 real bugs in graph_relevance/suggest_questions found in self-review graph_relevance() used a bare substring check with no word-boundary -- any short entity label ("AI", "US", "ROC") matched inside an unrelated longer word ("explain", "custom", "process"). Switched to compiled word-boundary regex, precomputed once per call instead of once per text (also fixes an O(texts x labels) re-lowering cost). Also dedupes case-variant labels ("Neural Networks"/"neural networks" from two different documents) so one concept isn't double-counted. suggest_questions() deduped by entity id, not label -- despite surprising_connections() two functions above it existing specifically because same-document-scoped ids mean the same real-world concept pair gets a different id per document. The same question text came out once per paper discussing it, not once total. Fixed to dedupe by normalised label pair, same as surprising_connections already does. None of this was caught by the original test suite or self-audit, because every original test was shaped like the code's own assumptions (small single-label fixtures, whole-word test strings, single-document fixtures) -- confirming internal consistency, not correctness. Pattern persisted to the review checklist. * Fix a second self-review round: plain \b fails on punctuation-edged labels The previous fix for the substring false-positive bug switched to plain \b word-boundary regex -- but \b requires a \w/\W transition, so it silently fails to match any label that itself starts or ends with punctuation: ".NET", "Ph.D.", "C++", "e.g.", "U.S." never matched even when the text said them verbatim. Switched to (?<!\w)...(?!\w), which asserts "not adjacent to a word character" regardless of what the label's own edge characters are. Caught by deliberately probing the fix's own edges in a second adversarial pass, not by the tests written alongside the first fix -- persisted to the checklist as its own lesson: fixing one instance of "tests shaped like my own assumptions" doesn't fix the habit. * Rank suggest_questions by confidence_score, nudged by endpoint degree Confidence + degree (moderate weight, DEGREE_WEIGHT=0.15 via log1p so a mega-hub can't drown out a confident edge between two obscure entities), per user's decision after weighing options. Both signals were already extracted per-edge/entity data, unused by this tool despite surprising_connections ranking by the same confidence_score field two functions above it. The one-per-document diversity guarantee is preserved deliberately: the guaranteed-slot selection (each document's best-ranked edge) and the leftover fill are ranked independently and never re-sorted together -- doing so would let a document with many high-ranked edges push a document with exactly one modestly-ranked edge out of the result entirely. Two of the three new tests initially passed on both old and new code for unrelated reasons (insertion-order coincidence; a guarantee that was never actually at risk from that test's shape) -- caught by checking each test against a plausible wrong alternative, not just against pre-fix history. Persisted as its own checklist lesson. * Fix 5 real bugs found by the code-review skill, reject 2 unverified findings 1. suggest_questions() deduped cross-document concept pairs during the raw scan, before ranking existed -- whichever document Kùzu visited first won the slot regardless of confidence_score, contradicting the function's own docstring. Fixed: score every row first, then dedupe by keeping each pair's best-ranked row. 2. _upsert() wrote confidence_score/weight via `e.get(k, default) or default` -- 0.0 is falsy in Python, so a genuinely-zero score was laundered into 0.5 (or 1.0 for weight) before it was ever persisted. This defeated the ranking fix above at the write side; proven by a test that failed even after the read-side default was fixed correctly. 3. suggest_questions()' degree count didn't exclude self-loops -- a self-referential edge is emitted twice by the undirected scan with the same id on both ends, inflating that entity's degree by +2 for zero real connectivity. 4. zotero_items_relevance() interpolated item.title with no None-guard, unlike item.abstract_note on the same line -- a titleless item scored the literal string "None" against KG entity labels. 5. graph_relevance() recompiled every cached label's regex from scratch on every call; a caller can invoke it several times per page load (one call per item batch). Patterns are now precompiled once per _refresh_entity_labels() cycle instead. Rejected as unverified: a claimed whitespace-run dedup gap in label matching (doesn't reproduce -- two label variants with different internal whitespace can't both match the same single occurrence of text); a claim that docs/diagrams weren't regenerated (verified false, zero diff, no diagram source references any of the new tool names). Persisted as new checklist items: ranking added after a filter existed means every pre-existing filter needs re-auditing for order-dependence; a falsy-default fix on one side of a field (read) doesn't cover the other side (write) of the same bug. * Harden every KnowledgeGraphClient method against a malformed response Previously each method only degraded on requests.RequestException (network-level failure) -- a Pydantic ValidationError on model_validate(), or an AttributeError/TypeError from .get() on a response that parsed as JSON but wasn't the expected shape, propagated as an unhandled exception through this client into whatever called it. Fixed consistently across every method (not just graph_relevance, which is where this was first flagged and deliberately deferred to be fixed everywhere at once) via a shared _safe() wrapper, each call site keeping its own already-defined degrade value. _get()/_post() also now catch a non-JSON 200 body (json.JSONDecodeError, a ValueError subclass) -- the HTTP call succeeds but resp.json() itself fails, the same 'reachable but unusable' class _safe() covers one layer up. 6 new regression tests, one per distinct degrade shape (list, single object, bool via .get(), the unreachable-KGStatus default, and graph_relevance's length-preserving zero-score degrade), plus the non-JSON-body case at the _get() level. All confirmed red on pre-fix code. * Close 2 loose ends in the malformed-response hardening pass + a test coverage gap _safe()'s except tuple didn't include ValueError, so clear_dead_letters()'s int(data.get("removed", 0)) still raised uncaught on a non-numeric field -- the exact bug class that commit existed to close, just missed for one conversion. graph_relevance() also had no length-mismatch check: every item in a wrong-length response can validate individually with nothing raised, silently breaking the positional-correspondence guarantee the surrounding comment promises. Separately: suggest_questions was missing from two pre-existing parametrized tests that exist specifically to catch a broken marker/regex for every tool (test_new_tools_advertised_in_system_prompt, test_tool_call_re_matches_new_markers) -- found by grepping the test file for the marker, not by writing suggest_questions' own tests. * Add missing /graph/suggest_questions route; fix a Zotero listing request race graph_routes.py's own docstring states every Phase-A Kùzu-backed capability gets a public route here regardless of whether it also has a chat ToolSpec -- suggest_questions had a client method, a kg_app.py internal route, and a ToolSpec, but no public route, making it unreachable outside the chat loop. Added, matching the existing pattern exactly (same DEFAULT_TOP_ENTITIES/SUGGEST_QUESTIONS_MAX bound as /graph/god_nodes and /graph/surprising_connections). Note: read_source has the same pre-existing gap from an earlier PR, not touched here. loadZoteroItems() had no request sequencing between overlapping calls (collection click, debounced search, and now the relevance toggle) -- pre-existing, but the relevance toggle's kg-round-trip path has a materially different latency than plain /items, making the race meaningfully more likely to surface a stale/mismatched item list. Fixed with a last-request-wins sequence counter. Two findings from this review round rejected: batching graph_relevance calls sequentially in zotero_routes.py is real but not fixed here -- concurrent HTTP calls would require a broader move off the synchronous requests library, out of scope for a review-fix pass. Compiling regex patterns under the lock in _refresh_entity_labels() is correct as-is, not a bug -- the suggested fix would reintroduce the exact compute-and-publish-must-be-atomic race the checklist's own item 4 exists to prevent for a background-refreshed cache. * Fix a dead docs/concepts/footnote.md link left over from the ontology rename Same TODO.md line as this branch's docs/ontologia.md -> docs/ontology.md rename also carried an adjacent dead link -- footnote.md was renamed to claim.md in an earlier, unrelated PR (the Footnote/FootnoteRelation -> CitedClaimNode/InferenceNode split), and nothing caught the leftover reference since it wasn't part of that rename's own scope. * Fix 3 real Copilot findings; document a 4th as the same already-deferred issue - openStream() bypassed loadZoteroItems() entirely (direct fetch, no zoteroRequestSeq participation, ignored zoteroSortByRelevance) -- opening a stream while relevance-sort was active silently showed unsorted, unbadged results, and could race with an in-flight loadZoteroItems() call with no coordination. Routed through the shared helper instead of duplicating the fetch. - The relevance match-count badge never rendered for a genuine zero-score item (0 is falsy in Svelte's {#if}) -- same falsy-zero defect class already fixed twice this session in Python, now caught in the UI too. Checked !== undefined instead. - The relevance toggle button had no aria-pressed, so its active state was only conveyed visually. The 4th finding (a chat-tier-drifted edge's source_file being citable even after its endpoints' trust_tier relabels non-chat via an id collision) is real, but it's the same already-known, already-deferred issue TODO.md tracks as 'KG entity ids aren't directory-unique' -- god_nodes/surprising_connections already share the identical exposure, suggest_questions just inherits it. Documented the citability angle there rather than a one-off fix in this one function. * Recalibrate the chat-tier-drift TODO.md entry -- it's inert, not urgent Chats never reach the knowledge graph at all today (.sess, not .md/.txt -- KnowledgeGraphService._full_index() only walks index_extensions), so trust_tier=='chat' never actually occurs on a real entity/edge. And the KG's trust_tier filter was never what enforced 'chats aren't sources' in the first place -- that's independently guaranteed by chats' storage format, ChromaIndexer's explicit chats/ exclusion, and RECALL being the one deliberately separate, non-citable path for chat content. This is a gap in a redundant backup layer for a rule two other mechanisms already hold, not a threat to the rule itself. My first write-up of this (previous commit) overstated it as a live citability leak. * Rewrite TODO.md as a lean backlog, not a project log 1610 -> 109 lines. TODO.md had become a chronological engineering journal spanning months -- resolved/historical narrative mixed in with genuinely open items, most checkboxes already stale by the time this session started (several already fixed by PRs #104/#105/#106, confirmed against the actual codebase rather than trusted at face value). Kept only what's still genuinely pending, verified against current code for each ambiguous item (chat-tier label, deletion cascade, graphify migration, streaming, container build, chart wiring, etc.) rather than assuming the checkbox state was accurate. Also dropped personal/private details incidental to the historical narrative (nothing left that belongs in a public repo's backlog). Cross-references to now-removed TODO.md sections fixed where they made an active, findable claim (config_reload.py, its test, ADR-004, ADR-008, ADR-015's header) -- either redirected to the code/ADR that's the real durable home, or the essential point inlined directly since the pointer was the only thing carrying it. Git history has the full removed content if any of it is ever needed again; docs/logs/ remains the home for investigation-log-shaped writeups, which is what the user asked TODO.md to stop doing. * Remove orphaned code fragment left over from a botched 2026-07-27 edit ADR-004 (2025-09-14) originally had a code block demonstrating an early, since-superseded CLI design where stream updates would run as tracked SQLite jobs (status/results commands checking job state) -- never actually implemented; streams still run synchronously today, no job system exists. The 2026-07-27 'CLI minimized' follow-up section got inserted in the middle of that original code block, swallowing its opening fence and heading but leaving the tail two function stubs + the orphaned closing fence dangling right after the new prose. Removed -- nothing here described anything that was ever built.

CServinL added 4 commits September 10, 2026 19:44

Drop the redundant min_length=1 on _RawWarrant.text

46ddd30

_reject_blank's not v.strip() check already rejects "" (and now ""'s superset, whitespace-only text) -- min_length=1 caught nothing the validator didn't already cover, just dead weight sitting next to it.

CServinL requested a balanced review from Copilot September 11, 2026 06:46

Copilot started reviewing on behalf of CServinL September 11, 2026 06:47 View session