Skip to content

Populate Toulmin qualifier/warrant/rebuts from the FOOTNOTES_JSON self-report - #105

Merged
CServinL merged 15 commits into
mainfrom
phase-b-toulmin
Sep 12, 2026
Merged

Populate Toulmin qualifier/warrant/rebuts from the FOOTNOTES_JSON self-report#105
CServinL merged 15 commits into
mainfrom
phase-b-toulmin

Conversation

@CServinL

Copy link
Copy Markdown
Owner

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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread prisma/agents/chat_agent.py
_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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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

Copy link
Copy Markdown
Owner Author

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

  1. rebuts accepting non-integer values (chat_agent.py:148) — rebuts was plain int, so Pydantic's lax mode silently coerced "rebuts": true1 and "rebuts": 1.01, 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.

  2. 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

Copy link
Copy Markdown
Owner Author

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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread prisma/agents/chat_agent.py
Comment thread prisma/services/chat_tools.py Outdated
… 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread prisma/services/chat_tools.py Outdated
Comment thread ui/src/routes/+page.svelte Outdated
Comment thread docs/concepts/claim.md Outdated
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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread ui/src/routes/+page.svelte Outdated
Comment thread prisma/storage/models/vault_models.py Outdated
Comment thread ui/src/routes/+page.svelte Outdated
.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).
@CServinL
CServinL requested a balanced review from Copilot September 12, 2026 00:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread prisma/agents/chat_agent.py
Comment thread ui/src/routes/+page.svelte
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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread prisma/agents/chat_agent.py
Comment thread ui/src/routes/+page.svelte Outdated
CServinL added a commit that referenced this pull request Sep 12, 2026
* 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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.
@CServinL
CServinL merged commit 2146757 into main Sep 12, 2026
@CServinL
CServinL deleted the phase-b-toulmin branch September 12, 2026 03:55
CServinL added a commit that referenced this pull request Sep 12, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants