Rename ontology doc, fix TODO staleness, add suggest_questions + stream-triage-by-graph-relevance - #106
Open
CServinL wants to merge 8 commits into
Open
Rename ontology doc, fix TODO staleness, add suggest_questions + stream-triage-by-graph-relevance#106CServinL wants to merge 8 commits into
CServinL wants to merge 8 commits into
Conversation
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).
# Conflicts: # .claude/CLAUDE.md
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.
…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.
CServinL
changed the title
…view
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.
…abels 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.
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.
…indings 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bundles several small/medium pending items together on this branch (user's call, to avoid micro-PRs) rather than as separate PRs:
Rename
docs/ontologia.md→docs/ontology.md— English-only in a public repo; the content was always English, just the filename was Spanish. Every reference updated across concept docs,TODO.md, ADR-017,.claude/CLAUDE.md, andvault_models.py'sCitedClaimNodedocstring; regenerated the 3 committed JSON schemas that embed that docstring.TODO.mdstaleness fix —god_nodes/surprising_connections/expand_node/read_source/per-section extraction were all still marked[ ]/"not built yet" despite shipping in PR Add knowledge-graph retrieval tools: expand_node, god_nodes, surprising_connections, authors, vault_health, timeline, read_source #104.Phase B, increment 2 —
suggest_questionschat tool. Same 4-layer patterngod_nodes/surprising_connectionsalready use (kg_queries.suggest_questions→KnowledgeGraphServicecache →kg_app.pyroute + client →chat_toolsToolSpec/handler). Phrases a grounded question perRelatesToedge, deduped by entity pair, capped at one question persource_filebefore filling remaining slots — so no single document crowds out the rest.Phase B, increment 3 — stream triage by graph relevance (lightweight design). A pure text-overlap score (
KnowledgeGraphService.graph_relevance, backed by a cacheddistinct_entity_labels()scan) against entity labels already extracted into the knowledge graph — no stream/Zotero content is ever indexed into Kùzu, keepingdocs/concepts/stream.md's stated "streams shouldn't pollute the knowledge graph" boundary intact. NewGET /zotero/items/relevanceendpoint scores and sorts the existing item listing; a new "Sort: Relevance" toggle in the Zotero browser panel calls it and renders a small match-count badge per item.Phase B's 4th increment, cross-chat entity linking, is explicitly deferred to its own session — it has no prior design anywhere in the repo and would require either extending KG entity extraction to chat content (reversing the deliberate "chats are not sources" decision) or inventing a new chat-scoped entity concept, on top of the cross-chat
RECALLmechanism that already does topical-similarity search across chats.Full suite: 1351 passed. UI build clean. Every new test confirmed red on pre-change code before the fix.