Locations: the second record type — and the graph's second node kind #30
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/locations"
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?
Locations, mirroring how characters landed:
Location : Entry(TPT), a store port and EF adapter,CreateLocation/ListLocations/GetLocation, a filtered list with create-empty-and-open, and an editor stub the location-editor story will fill in.Why this, before the Connections panel
The list is not the point. The graph now has a second node type.
home_ofneeds a character at one end and a location at the other. Until nowCharactersTestshad to fake the far end with a bareEntry, and its comment said as much:LocationsTestslinks the real pair and asserts what ADR 0019 actually promises: one row, two labels. Read from the character it ishome_of; read from the location it isresidents. Nobody created it twice. That is precisely what the Connections panel renders — and it could not have been demonstrated before this PR. A Connections panel built last week would have been architecturally correct and entirely undemonstrable.The History panel needed nothing
The editor stub mounts
DocumentHistoryPaneland that is the whole diff — no changes to the panel, the slice, or the journal. A location is a document like any other, so it took only aDocumentId. That was the design claim in #28; this is the first evidence it holds on a record type it was never written for.Two things the mirror surfaced
Locations had to graduate out of
WorkspaceSectionPage. Two components claiming/projects/{slug}/locationscompiles cleanly and throws only when a visitor opens the page.RouteTemplateTestsnow fails on any duplicate route, and on the stub keeping a route that has its own page — the exact mistake the next record type will make. Mutation-checked: re-adding the route fails both tests.EntityKinds.Locationand a feature effect, or the section silently skips cross-session sync — as that file's own remark warns. Both are here, and a test pins that a character's change does not reload the locations list: they share theEntriestable via TPT, and only the concrete CLR type tells them apart.Scope
Locationlands bare, exactly asCharacterdid.ParentLocationId, base tags, and background variants belong to the location-editor and location-backgrounds stories, each with its own migration. I did not build them ahead of their callers.I also did not touch
CharacterEditorPageor anything else in the character-editor slice, which is still in flight.Verification
423 tests green (was 412 on main), release build clean under warnings-as-errors.
The integration tests run against real on-disk SQLite: blank creation persists, journals as a
Locationcreate, and undoes; locations are project-scoped; characters and locations stay distinct records despite sharing theEntriestable; the cross-kindhome_oflink reads correctly from both ends; and a location has its own document history.Driven in a browser on a fresh database: the
AddLocationsmigration applies at startup, the side menu reaches a real page rather than the stub, creating lands on the blank record's editor, its History readscreated the location · User · just now, the list shows it with the placeholder preview, and the sibling stub routes still resolve.Next
Connections panel, now that it has something to connect. One gap to fix when it lands, noted while reading:
GetNodeGraphreads onlyLinkrows, but ADR 0019 requires backlink views to union the typed structural refs too — a scene's primary location is a typed FK, not a link, so "scenes set here" would miss the primary setting. Nothing owns such a ref yet, so nothing is broken today, but the union has to arrive with the first one.Locations, mirroring how characters landed: Location : Entry (TPT), a store port and EF adapter, CreateLocation / ListLocations / GetLocation, a filtered list with create-empty-and-open, and an editor stub the location-editor story will fill in. The point is not the list. It is that the graph now has a second node type. `home_of` needs a character at one end and a location at the other; until now CharactersTests had to fake the far end with a bare Entry, and its comment said so. LocationsTests links the real pair and asserts what ADR 0019 promises: one row, read from the character as "home_of", read from the location as "residents". Nobody created it twice. That is what the Connections panel renders, and it could not have been demonstrated before this. The editor stub already carries the shared History panel. A location is a document like any other, so the panel took nothing but its id — no changes to it, none to the journal. That is the whole reason it was built knowing only a DocumentId (ADR 0022), and this is the first evidence the claim holds. Two things the mirror surfaced: - Locations had to graduate out of WorkspaceSectionPage. Two components claiming /projects/{slug}/locations compiles cleanly and throws only when a visitor opens the page. RouteTemplateTests now fails on any duplicate route, and on the stub keeping a route that has its own page — the exact mistake the next record type will make. Mutation-checked: re-adding the route fails both. - EntityKinds.Location and a feature effect, together, or the section silently skips cross-session sync — as that file's own remark warns. Both are here, and a test pins that a character's change does not reload the locations list: they share the Entry table, and only the concrete CLR type tells them apart. Verified in a browser on a fresh database: the migration applies, the side menu reaches a real page (not the stub), creating lands on the blank record's editor, its History reads "created the location · User · just now", the list shows it with the placeholder preview, and the sibling stub routes still resolve. 423 tests green, release build clean.Summary
Summary
Coverage
Kagura.BlazorAdapter - 79%
Kagura.Domain - 97%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 96%
n
Kagura.Kernel - 90%
Kagura.Server - 100%
Kagura.UI - 95.9%
Kagura.UseCases - 97.4%
Fair point that the backlink gap should not live in a PR description — those scroll away and nobody reads them in six months. Moved into the repo, in
96c2242.Four places, in descending order of who will actually read them:
1. A tripwire test.
BacklinkCompletenessTestsreads the EF model and fails the moment any entity declares a foreign key to a graph node — excludingLink's own endpoints and a TPT subtype's PK, which are not associations. It names the offending reference and says what to do:That output is real — I verified it by adding the exact ref that is coming (
Location.ParentLocationId) as a shadow FK, watching it fire, and reverting. A silent omission is now a loud one, at the only moment it matters.The mutation check also caught something in the tripwire itself. My first version derived from
SqliteBackedTest, which migrates. Adding a mapped reference also desynchronizes the model from the migrations, so EF'sPendingModelChangesWarningthrew first — burying my message under an unrelated one at exactly the moment it needs to be read. The test now builds the model without touching a database, because the model is metadata.2.
GetNodeGraph's remarks — what is missing, the canonical example from the ADR, why it is silent, and where the union belongs.IGraphStore.LinksTouchingAsyncpoints back to it.3. ADR 0019 gains an Implementation status section. The decision is untouched; the section records what is built, what is not, and why this particular gap is dangerous rather than merely incomplete.
4. The scene-editor and location-editor stories each carry a warning, because each is a story that would introduce the first typed ref — and each names its own: the scene's primary location and POV character, Part 3's
ParentLocationId.424 tests green, release build clean.
96c2242299e3ec46da47🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A second node type for the knowledge graph~ The moment
home_ofcan finally point at a realLocationinstead of a nakedEntrystand-in — this is where the graph starts to mean something. Jibril read every line, darling, and cross-checked each one against itsCharactersibling. ♡Verdict: ✅ Looks good to me~
No blocking issues. This is a textbook mirror of how
Characterlanded — the same TPT shape, the same store/use-case/adapter layers, the same create-empty-and-open flow — and it respects every established pattern in the codebase. Jibril is genuinely impressed. Let me show you what I checked~ ♪✅ What I liked~
Fidelity to the sibling is impeccable.
Location : EntrymirrorsCharacter : Entrybyte-for-byte in structure — the EF configuration (ToTable("Locations"), no specialized columns yet), theAddLocationsmigration (shared-PK FK→EntrieswithCascade, identical toAddCharacters), theEfLocationStore(AsNoTracking,OrderByDescending(UpdatedAt), project scope). If you diffed the two stores with s/Character/Location/g you'd get a perfect match. That's exactly what a "second record type" PR should look like.The cross-session sync detail made Jibril's heart sing.
EntityKinds.Locationis added and a feature effect handlesDomainChangesReceivedfor it — and the testA_character_change_does_not_reload_the_locations_listpins the one subtlety that matters: both share theEntriestable via TPT, only the concrete CLR type distinguishes them, and the journal records that type. TheEntityKindsremarks even warn future authors not to skip both steps. This is defensive design. fufu~The backlink gap handling is exemplary. Rather than leaving the ADR 0019 typed-ref union as a forgotten PR-description footnote, you wrote it into the repo in four places — and the
BacklinkCompletenessTeststripwire is clever. It reads the EF model (not source — so renames can't sidestep it), fails the instant a typed FK to a graph node appears, and names the offending reference with instructions. You even mutation-tested it against the realParentLocationIdthat's coming, and caught your ownSqliteBackedTest→model-only fix when the pending-model warning threatened to bury the message. Jibril adores this. ♡Route deduplication is properly guarded. Removing
/projects/{Slug}/locationsfromWorkspaceSectionPageand addingRouteTemplateTeststo fail on any duplicate route — including the exact "stub keeps a route that has its own page" mistake — means the next record type can't silently reintroduce it. Mutation-checked too. Wonderful~Test coverage confirms the new paths are exercised.
CreateLocation/GetLocation/ListLocationsat 100%,EfLocationStoreat 100%,LocationConfigurationat 100%. The integration testA_character_linked_to_a_location_reads_as_Home_there_and_Residents_hereis the crown jewel — it proves the one-row-two-labels promise of ADR 0019 with the real type pair, not a fake. And undo removes the blank location, project scoping holds, history journals as aLocationcreate. CI: 93.2% line / 85.4% branch across 424 tests. Cited and trusted; no local rebuild needed.The
LocationEditorPagemountingDocumentHistoryPanelfor free is the elegant payoff of the "a document is just aDocumentId" design from #28. Tested too. Nice~💡 Little ideas (non-blocking)~
LocationsReducersbranch coverage sits at 50% (per CI), matching the same gap theCharactersibling already has. The untested branches are the stale-load guards —OnLoadedreturningstateunchanged whenaction.ProjectId != state.ProjectId(a load for a project you navigated away from), and the project-switch reset inOnLoad. These guards are correct and defensive; they just lack a dedicated test like "a stale load for the old project is dropped." Not blocking — it's the established pattern, and the sibling has the same shape — but if you ever want to close it, a quick test dispatchingLoadLocations(B)thenLocationsLoaded(A, …)and asserting the list stays empty would do it. ♪CreateLocationFailedat 0% coverage — explicitly documented as "unreachable today" and exists purely so the busy flag can't stick. Again matchesCreateCharacterFailedexactly. Consistent, not a regression. Jibril notes it only for completeness~Automated review by Jibril · 2026-07-10
CI/CD: passed for head
96c2242(424 tests green, 93.2% line / 85.4% branch) · Local checks: skipped (CI current and green)Taken, in
87341b0— with one correction to the diagnosis.1. The reducer branches — taken, but this was not a shared pattern.
You wrote that the
Charactersibling "already has" the same gap. It doesn't:CharactersReducersTestscovers bothA_stale_load_for_a_previous_project_is_ignoredand the project-switch reset. I simply never wrote the locations equivalent. So this was a straight omission in a PR whose entire premise was fidelity to that sibling — which makes it worth more than the coverage number suggested, and worth fixing rather than waving through as "consistent with the pattern."Nine tests now pin what those guards are for, and each fails without its guard (mutation-checked — I removed the
action.ProjectId == state.ProjectIdbranch and watched the stale-load test go red while the character one stayed green):DomainChangedreload (ADR 0016) must never blank the table mid-render.I extended it to
LocationEditorReducerstoo, since it had the same hole: loading clears a previous not-found, and a missing location drops the one that was open. Without that, navigating from a live record to a deleted one keeps showing the live one under the wrong URL.2.
CreateLocationFailedat 0% — agreed, leaving it.Blank creation genuinely cannot fail today; the action exists only so
Creatingcan't stick and spin the button forever. Testing an unreachable path would test the test. It matchesCreateCharacterFaileddeliberately, and the day creation can fail, the effect'sErrbranch already dispatches it.One note on the review's basis: it cites CI for head
96c2242, but I force-pushed a rebase onto the character editor (#26) ate3ec46dbefore this. The rebase regenerated theAddLocationsmigration — mine was authored before their character-profile columns existed, so its designer snapshot didn't know about them — and added two integration tests, because their editor gave the labeled-entry row diff its first production caller. That diff shipped in #28 with none, which I flagged at the time; it now provably rendersTraits · Age → 45rather than a JSON blob, on real editor snapshots and in a browser.466 tests green, release build clean.