Knowledge-graph foundation: Entry nodes + Link edges + backlinks #4
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/graph-foundation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The structural heart of the knowledge base (ADR 0019): every linkable thing is an
Entrynode, the open-ended associations are genericLinkedges, and opening any node yields its backlinks — "open X, see everything using it" from one indexed query.What's in it
Domain (
Kagura.Domain.Graph, pure)EntryKind— the uniform discriminator (Character, Location, Scene, LoreEntry, Cg, TimelineEvent, Chapter, Route, Asset).Entry— base node:Id,ProjectId,Kind,Title,Description, timestamps. Non-sealed so concrete content types (Character, Location, …) will share its PK via table-per-type in later slices; until then it's a usable generic node.Link— directed edge:From/To/Role/Note/optionalAtTimelineEventId. Rejects self-links.LinkRole+LinkRoles— the small, extensible role vocabulary (home_of/residents,appears_in/features,set_in/scenes,depicts/depicted_in,references/referenced_by,uses/used_by, symmetricrelated_to) withLinkDirectionand inverse/symmetric label resolution.UseCases (
Kagura.UseCases.Graph)IGraphStoreport;NodeSummary/GraphEdgeView/NodeGraphViewread models (adapters get summaries, so a link across modules needs no compile dependency).LinkNodes— validates a known role, distinct endpoints, both nodes present, same project, and no duplicate; returns the created edge as seen from the source.GetNodeGraph— unions a node's inbound + outbound edges and groups them by the label shown at this end, so a directed edge surfaces under its inverse when the node is the target (a location shows residents; the character shows home_of).Infrastructure
Entry/LinkFluent configs +AddGraphEntryAndLinkmigration.Restrict: deletion stays app-managed (soft delete, ADR 0020), which also sidesteps SQLite's multiple-cascade-path error withLink's threeEntryFKs (from/to/at-timeline).(FromId, ToId, Role)index is the duplicate-edge safety net;EfGraphStoretranslates the realSqliteExceptionintoTryAddLink=falseinside the adapter — EF never leaks upward (same pattern as the project slug).ValueConverterto shared config (UtcTicksConverter) and repointedProjectConfigurationat it — as flagged when it landed.Tests — 51, all green (was 25; +26)
LinkNodesvalidations + same-pair-different-role,GetNodeGraphgrouping/inverse-label/sorting/empty/not-found.SqliteBackedTestso both integration suites share one migrated-SQLite fixture. TheEntry→ProjectFK again earned the real-DB strategy its keep — it caught tests that scoped entries to a non-existent project.Verification
dotnet build— 0 warnings / 0 errors (warnings-as-errors).dotnet test— 51/51 pass.Projects,Entries,Links), serves HTTP 200, clean log.Not in this PR (next slice)
Soft delete + the
ChangeLogjournal / undo (ADR 0020), link removal/edit/re-point, andRelationshipas a specialized edge (depends on theCharactertype). Backlink views will then also unionRelationshipand the typed structural refs (scene primary location / POV) per ADR 0019.🤖 Generated with Claude Code
acb5b20c97to1c6b7a3583Summary
Summary
Coverage
Kagura.Domain - 95%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.1%
Kagura.Kernel - 90%
Kagura.UseCases - 96.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ A knowledge-graph foundation! Entry nodes, directed Link edges, backlinks that surface under their inverse label... fufu~, the structure of this is beautiful ♡ The way
GetNodeGraphflips directed edges to their inverse label depending on which end you're standing on — that's elegant domain modeling. I read every line and traced every query. The architecture is clean and follows your established patterns faithfully.Verdict: ⛔ I can't let this pass just yet~ ♡
⛔ These need fixing before I'm satisfied~
Entryhas zero direct entity tests — andEntry.Edit()+ constructor guards +Normalizebranches are all untested.Your own CI coverage bot confirms it:
Kagura.Domain.Graph.Entrysits at 80.7% line / 50% branch — the worst branch coverage of any new class in this PR. TheEdit()method (lines 55-62), theArgumentException.ThrowIfNullOrWhiteSpace(title)guard in both the constructor andEdit, and theNormalizehelper's three branches (null→null, whitespace→null, content→trimmed) are never exercised by any test.Why this blocks: your sibling
Projectentity has a fulltests/Kagura.UseCases.Tests/Projects/ProjectTests.cswith dedicated tests for the exact same pattern —Rename_changes_the_name_and_advances_UpdatedAt,Rename_rejects_a_blank_name, and constructor blank-input rejection theories.Entryis the direct analog (same shape: identity fields,Edit/Renamemethod,Normalize/trim, timestamp stamping) and it has no test file at all. New public domain logic with branches, untested, contradicting the established convention — that's all three of my blocking criteria at once, fufu~Fix: Add
tests/Kagura.UseCases.Tests/Graph/EntryTests.csmirroringProjectTests.cs:Editchanges title/description and advancesUpdatedAtbut keepsId/ProjectId/Kind/CreatedAtEditrejects blank title (Theory:""," ",null)CreatedAt==UpdatedAtNormalize: descriptionnull→null," "→null," text "→"text"(the content branch is the one CI says is uncovered)While you're at it: the
Linkentity's constructor guards (ThrowIfNullOrWhiteSpace(role), thefromId == toIdself-link throw) are also untested at the entity level — they're defense-in-depth behindLinkNodes' pre-validation, so they never fire in the current suite (that's the uncovered branch inLink's 83.3%). A smallLinkTests.cscovering the constructor invariants directly would close that gap the same wayProjectTestsdoes. I won't die on this hill, but~ ♡💡 Little ideas (non-blocking)~
Normalizeis duplicated verbatim betweenEntry.csandLink.cs(identical 4-line private helper). Normally I'd flag DRY, but the domain layer is explicitly pure/dependency-free and this is a 3-line method — extracting a shared type would be over-engineering. Just noting it exists; if a third timestamped entity lands with the same helper, that's the signal to extract.Entry.Descriptionis unbounded (noHasMaxLength) whileLink.Noteis capped at 1000 andEntry.Titleat 300. IfDescriptionis meant to be free-form long-form, the lack of a cap is intentional and fine — but a one-line comment inEntryConfigurationsaying "deliberately unbounded: long-form lore/description" would stop a future reader from "fixing" it. (Your code comment says// free-form, unbounded— that's actually enough! Forget I said anything~ ♪)✅ What I liked~
EfGraphStoreadapter is a near-perfect mirror ofEfProjectStore— sameSqliteConstraintUniqueconst, same try/catch-then-Detachpattern so theDbContextstays usable after a constraint violation, same "never leak EF upward" discipline. When a PR's new adapter is indistinguishable in quality from its sibling, that's a happy Jibril~ ♡LinkNodesdoes not pre-check for duplicates — it relies entirely on the unique(FromId, ToId, Role)index viaTryAddLinkAsync. The PR description even calls this out explicitly. Correct and race-safe.GetNodeGraph.ToEdgeViewis genuinely clever and well-tested — a location sees "residents" (inbound edges under inverse label), the character sees "home_of" (outbound under forward label). The grouping/sorting is deterministic and tested with the["Hero", "Sidekick"]ordering assertion.UtcTicksConverterextraction — moving the UTC-ticksValueConverterfrom a private field inProjectConfigurationto a shared, reusable class is exactly the right refactor, and repointingProjectConfigurationat it keeps the change clean. No behavior change, just less duplication.Entrygap is the one blemish on an otherwise exemplary testing effort.Restrictwith a documented rationale (SQLite multiple-cascade-path + app-managed soft delete per ADR 0020). The migration, designer, and model snapshot are all three consistent.Automated review by Jibril · 2026-07-09
CI/CD: passed for head SHA
1c6b7a3(coverage bot: 95.7% line / 82.6% branch, 51 tests) · Local checks: skipped (CI green)Fair catch — I tested
Projectbut left its direct analogEntryuncovered. Fixed in570bd35.⛔ Blocker —
Entryentity testsAdded
tests/Kagura.UseCases.Tests/Graph/EntryTests.cs, mirroringProjectTests:Editchanges title/description and advancesUpdatedAtwhile keepingId/ProjectId/Kind/CreatedAtEditand the constructor reject a blank title (theories:""," ",null)CreatedAt == UpdatedAtDescriptiontrim/normalize across all three branches —null→null," "→null," lore text "→"lore text"(the content branch CI flagged)Also closed —
Linkconstructor guardsTook the nudge:
LinkTests.csexercises the invariants directly — blank-role rejection, self-link rejection, andNotetrim/normalize — the defense-in-depth guards behindLinkNodesthat the suite never fired.Non-blockers
Left as-is, per your own conclusions:
Normalizestays duplicated (3-line pure helper; extract only when a third entity wants it), andEntry.Descriptionkeeps its// free-form, unboundedcomment.+19 tests (45 → 64 unit; 70 total). Build clean (0 warnings), CI green on
570bd35(build ✅ / test ✅). Ready for another look.