feat(properties): relation property type — link notes, files, tasks and events - #942
Merged
Conversation
…fs, dot-path, gated rollups
Resolves memry://<kind>/<id> relation property URIs to display data (title, existence) for chip rendering. Spans both databases: notes from note_cache (index db), tasks and events from tasks/calendar_events (data db). Groups URIs by kind and issues at most one inArray query per kind regardless of input size. calendar_external_events is never queried per spec. Malformed/missing refs come back exists:false instead of throwing.
Renders relation property arrays as live-resolved chips via properties:resolveRefs (Task 7's batch resolver), one chip per URI with a kind icon and title resolved fresh on every mount/value change — no title is ever stored on the value. Dangling refs (exists: false) render as a distinct "deleted" chip and are kept in the value; only an explicit remove click writes, producing a new array via onChange without mutating the input. Dispatches from PropertyRow next to the multiselect branch. Read side only — picker lands in Task 10.
Write side of relation properties: a "+" trigger on RelationEditor opens an embedded Popover (tag-icon-chip pattern) hosting RelationPicker, a grouped search (Notes & Files / Tasks / Events) that appends a formatRelationUri() result to the value on select. Reuses searchService.quick + calendarService.searchEvents, the same channels the canvas add-card picker uses, rather than adding IPC. Unlike that picker, notes are not restricted to markdown so a relation can also point at a filed file. Duplicate URIs are blocked in RelationEditor before onChange ever fires, since property_refs would reject the row anyway. Also opts the new debounced search hook out of react-you-might-not-need-an-effect/no-adjust-state-on-prop-change, the same override already granted to use-canvas-add-search.ts for this exact fetch-driven pattern.
The app strips the default focus ring globally (*:focus-visible {
outline: none } in assets/main.css), so the "+" add-relation trigger's
focus:outline-none left keyboard users with zero visual indicator when
tabbed to it — WCAG 2.4.7. Matches the ring-1/ring-ring convention already
used on comparable icon triggers in this area (BacklinkCard,
comment-attachments) rather than inventing a new treatment.
Pinned with a test asserting the className carries a focus-visible:
treatment — a proxy for the real visual check, since jsdom doesn't
compute Tailwind styles.
Read-only compact chips for relation properties in the folder table and grouped table: existing refs show icon + title, dangling refs render muted like the note-side RelationEditor. No picker, no remove control, no click-to-edit — EditablePropertyCell bypasses its edit affordance for type 'relation' unconditionally so a click never falls through to the generic text editor and overwrites the URI array. Batches concurrent resolveRefs calls made within the same tick (e.g. a virtualized table mounting many relation cells in one commit) into a single IPC round trip via a module-scoped microtask coalescer, instead of one resolveRefs call per cell. Widens property-cell.tsx's PropertyType union with 'relation', which forced the two exhaustive Record<PropertyType, AppIcon> column-icon maps in grouped-table.tsx and folder-table-view.tsx to add an icon (Link2, matching the note-side property type icon).
RelationCell now memoizes its parsed uris off the raw value prop instead of receiving a fresh .map(String) array on every render of the PropertyValueDisplay dispatcher. Its resolve effect depends on that memoized array, so a re-render that leaves the underlying relation value unchanged (e.g. every visible cell re-rendering on a folder search keystroke via highlightQuery) no longer re-fires the effect and re-issues a resolveRefs call for data that never changed. Also fixes packages/contracts/src/property-types.test.ts, which never picked up RELATION: 'relation' when it was added to PropertyTypes on this branch.
getIncomingReferences unions wiki-link backlinks with note-targeted
property_refs rows, tagging property-sourced entries with
via: { kind: 'property', propertyName }. A note referenced both by
wikilink and relation property gets two distinct, labeled backlink
entries. Dangling property_refs rows (source note deleted from the
rebuildable index-DB cache) are filtered out before they reach the
panel.
getNoteLinks (vault/notes-queries.ts) now calls getIncomingReferences
and threads `via` through to the Backlink IPC shape. The backlinks
panel renders property-sourced rows as "<property> → <source title>".
…sources
A source note that references the target both via [[wikilink]] and via a
relation property now produces two Backlink entries with the same
sourceId (by design — see prior commit). Both page transforms
(note.tsx, journal.tsx) used sourceId as the React list key
(BacklinksSection.tsx renders key={backlink.id}), so the two sibling
BacklinkCards collided on key.
Confirmed empirically (not just by inspection): reproducing the
colliding-key shape and forcing a sort-triggered re-render swapped the
two cards' isExpanded state — collapsing one card visibly expanded the
wrong one. noteId (used for navigation/selection via
handleBacklinkClick) is untouched; only id (used solely as the list
key) is now suffixed with the property name when via is set, keeping
it unique per reference rather than per source note.
…mula Round 1 fixed the React key collision by computing id inline in both note.tsx and journal.tsx, but nothing exercised those lines directly: note.test.tsx and journal.test.tsx both stub out BacklinksSection entirely, and the new BacklinksSection.test.tsx only proves the component isolates state given already-unique ids. Extract the formula into backlinkId(sourceId, via) in components/note/backlinks/types.ts, unit-test it directly, and have both pages call it instead of duplicating the ternary. Also unmock the real backlinkId export in both page test files (importOriginal) and add one test per page that renders two backlinks sharing a sourceId (one via a wikilink, one via a relation property) and asserts the computed ids differ — this exercises the actual transform lines, not just the extracted function. Verified by temporarily reverting each page's `id: backlinkId(...)` line back to `id: bl.sourceId` and confirming its guard test fails with the exact duplicate-id assertion (plus React's own duplicate-key warning), then restoring and confirming green.
Surface note-to-note relation property refs as graph edges. Widens GraphEdge['type'] to include 'relation' and keeps the hand-maintained preload declaration in sync. Task/event refs and refs to notes absent from note_cache are excluded, mirroring the existing wikilink guard. Renderer edge styling (color) is intentionally left untouched pending a design decision - see task-13-report.md.
Revised design ruling after the dashed-edge approach was found disproportionate (sigma is WebGL-only, no built-in dash program, no prior custom EdgeProgram in this codebase): relation edges keep the wikilink grey but render at size 1.25 vs wikilink's 2, distinct from the unregistered EDGE_SIZES fallback of 1 so the treatment is pinned by a real assertion, not the fallback.
A relation added through the UI was persisted as `text` and its value later flattened to a JSON string in the vault file. `usePropertySection` seeds a new Relation with `getDefaultValueForType`, which is `[]`, and no type crosses `properties:set`. `isRelationValue([])` is false, so the projector inferred `text` and `ensurePropertyDefinition` wrote that definition row. `getPropertyType` then returned the stored type forever without re-inferring, so `deserializeValue(value, 'text')` handed back a raw string: on reopen the row rendered as literal text, and the next property edit round-tripped that string into YAML, deleting the `property_refs` rows, the graph edge and the backlink. `getPropertyType` now prefers `relation` whenever the value is structurally an array of `memry://` URIs. Read-time only: the definitions table is a derived cache of `.memry/properties.md`, and `PropertyDefinitionSchema` has no relation member, so persisting one would make that file fail to parse and drop every definition in it. Deriving from the value also self-heals notes already damaged, as long as the YAML array survived. Tests reproduce the real sequence — default-value write, then populated write, then an edit to a different property — and `property-ref-queries.test.ts` now passes the production `getPropertyType` wiring instead of raw `inferPropertyType`, which had let it stay green with the defect fully present.
`main.css` clears the global `*:focus-visible` outline, so a keyboard user tabbing across relation chips saw no indication of which remove button was focused (WCAG 2.4.7). Same treatment the add trigger fifteen lines below already carries.
Main demonstrably returns `type: 'relation'`, but the unions declared in `preload/index.d.ts` and `packages/rpc/src/notes.ts` stopped at `'rating'`. Any `Record<PropertyType, X>` or exhaustive switch built on either would compile clean while silently omitting relation. Four other copies of this union were already widened; these two were left factually false.
…ippets `getNoteLinks` ran the `[[target]]` context scan for every incoming ref with no check on `ref.via`. A source that both wiki-links a note and points at it from a relation property produced two cards, and the property card carried the wiki link's snippets and a mention count that belonged entirely to the wiki link — the same excerpts shown twice, under a label that did not produce them. Context extraction is now skipped when `ref.via` is set. With no mentions to reveal, `BacklinkCard` also stops rendering the expand chevron, which previously toggled nothing.
The shared add-property popup listed Relation, and `template-editor.tsx` then mapped it to `'text'` after the fact — the user picked Relation and got a plain text box with no indication anything had changed. Same silent-downgrade class as the definition-row defect. `InfoSection`/`AddPropertyPopup` take an `excludeTypes` list, and the template editor excludes `relation`. Template storage is unchanged; supporting relations there is a separate feature.
The spec claimed relation values "round-trip unmodified back through that client's pushes" on an older build. Half true, and the wrong half was stated as settled. Passive sync is lossless — pull writes YAML verbatim, push re-reads it from the file, so the index-DB copy never reaches the wire. But the moment a user edits any property on that note in a released build, `use-properties.ts` rebuilds the record from index-DB values where the array has already been flattened to a JSON string, and that string is written to YAML and pushed. Recorded alongside: this is a pre-existing class, not a new failure mode — property definitions never sync, so an existing multiselect array flattens identically on another device. The "verification item before release" now names writing rather than rendering as the risk. Added the gap the spec never asked about: on the originating device the first write is `[]`, which no inference can type. The user guide said relation chips are clickable and open the target. They are not; the claim is replaced with what the chips actually do.
The structural override added to `getPropertyType` covers the note projector — local edits, externally-authored notes, re-indexing, sync creates. It does not cover sync updates. `setNoteProperties` has exactly two production callers, and the second is `noteHandler.applyUpsert`, whose own `getType` closure was still "canonical definition wins, else infer". That path is reachable and self-propagating. A relation's first write is `father: []`; whichever device indexes or receives it pins the canonical definition to `text`, and neither `vault/note-sync.ts` nor this closure ever corrects an existing row. Every later sync update then typed the populated value as `text`, `deserializeValue(value, 'text')` returned the raw JSON string, and because both push builders serialize properties from the index DB — not from the file — that device shipped the flattened string to every other device on its next push. A body edit, tag change or rename was enough; no property edit required. The file `applyUpsert` writes is passed to `markWritebackIgnored`, which the watcher honours, so the fixed projector never re-typed it. The closure moves to `resolveSyncPropertyType` so a test can exercise the exact function production uses against real databases rather than a re-implementation. It short-circuits on `isRelationValue` and deliberately skips `saveCanonicalPropertyDefinition` on that branch: `PropertyDefinitionSchema` has no `relation` member, so nothing may write that type into a definition store. The mutation run shows why that skip is load-bearing — without the guard, inference derives `relation` from the populated array and persists it.
The focus-visible ring on the chip remove buttons had no test, so a className regression would have been silent. Mirrors the existing add-trigger assertion, and covers both chip states — they branch for hover styling and could drift.
The previous correction replaced one false claim with another. It said push "re-reads it from the file with extractProperties(frontmatter)" and that "the index-DB copy never reaches the wire" — that conflated two files sharing a basename. `vault/note-sync.ts` does call `extractProperties`, but for indexing. The push builders are `sync/note-sync.ts:95` and `sync/item-handlers/note-handler-sync-helpers.ts:63`, and both build properties from `propsToRecord(getNoteProperties(indexDb, ...))`. Only content and tags come from disk. The truth is worse than what was written. On an old client the relation is already a flattened string in the index DB the moment it is pulled, so any push of that note ships it — body edit, tag change, rename, dirty-recovery reseed. "Lossless" holds only while that device never pushes the note at all. A property edit is the fastest route, not the only one. The verification item now says to test an ordinary edit rather than a property edit. Also records both structural overrides under the empty-default bullet, and why the sync-update one is not optional cleanup.
# Conflicts: # apps/desktop/src/renderer/src/pages/template-editor.tsx # packages/i18n/src/locales/en/notes.json
github-actions
Bot
added
dependencies
documentation
|
React Doctor found 1 new issue in 1 file · 1 error · score 82 / 100 (Needs work) · 1 fixed · vs Errors
Reviewed by React Doctor for commit |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Chips need a note's own emoji to render it in place of the kind icon, and navigation needs two ids the resolver did not carry: a task's project (to scope the tasks list) and an event's start (the calendar cannot focus an event until its range has moved to the date that contains it). All three come from columns already on the tables the resolver queries, so this stays at three queries with no new joins.
…rget Chips render the target note's own emoji instead of the generic kind icon, and clicking one opens what it points at. One hook serves both surfaces so the note property row and the folder-view cell navigate identically. Each kind rides a viewState contract the destination already honours: notes/files open a tab keyed by entityId; tasks send openTaskId plus selectedProjectId, the pair journal-day-panel already uses; events send focusCalendarEventId, focusDate and focusedAt together, because the calendar's first effect needs the date to move its range before its second effect can find the event, and the token is what makes a repeat click work. The chip becoming a button forced the remove control out of it — a button inside a button is invalid markup — so the two are now siblings. Dangling refs stay inert text, and in the folder view the click is stopped from reaching the surrounding row.
The calendar's anchorDate is a local YYYY-MM-DD string. parseLocalDate
splits it on '-' and feeds the parts to new Date(y, m, d), so the full ISO
timestamp this was passing produced Number('30T12:00:00.000Z') = NaN, an
Invalid Date, and the range memo threw RangeError: Invalid time value —
taking the whole calendar tab down through the error boundary.
toLocalDateKey converts through a real Date so an event late in the day
lands on the local day the user sees it on, not its UTC day. An unparseable
instant now omits focusDate entirely, which opens the calendar unfocused
instead of crashing it.
The previous test asserted the raw ISO value, so it encoded the bug rather
than the contract the calendar actually requires.
# Conflicts: # apps/desktop/src/main/database/queries/notes/property-queries.ts # apps/desktop/src/main/database/queries/notes/query-helpers.ts # apps/desktop/src/main/sync/item-handlers/note-handler.ts # apps/desktop/src/renderer/src/components/folder-view/folder-table-view.tsx # apps/desktop/src/renderer/src/components/folder-view/grouped-table.tsx # apps/desktop/src/renderer/src/components/folder-view/property-cell.tsx # apps/desktop/src/renderer/src/components/note/ghost-affordance-row.tsx # apps/desktop/src/renderer/src/components/note/info-section/AddPropertyPopup.tsx # apps/desktop/src/renderer/src/components/note/info-section/InfoSection.tsx # apps/desktop/src/renderer/src/components/note/info-section/PropertyRow.test.tsx # apps/desktop/src/renderer/src/components/note/info-section/PropertyRow.tsx # apps/desktop/src/renderer/src/components/note/info-section/editors/index.ts # apps/desktop/src/renderer/src/components/note/info-section/types.ts # apps/desktop/src/renderer/src/lib/property-utils.ts # apps/docs/src/user-guide/notes/properties-tags.md # packages/contracts/src/property-types.test.ts # packages/contracts/src/property-types.ts # packages/i18n/src/locales/en/notes.json # packages/rpc/src/notes.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <button | ||
| key={item.id} | ||
| type="button" | ||
| role="option" |
There was a problem hiding this comment.
React Doctor · react-doctor/role-has-required-aria-props (error)
Screen reader users can't tell the state of this option without its required ARIA props, so add aria-selected.
Fix → Add every required aria-* attribute so assistive tech can expose the role's state correctly.
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
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.
What
Adds a
relationproperty type: a note property whose value references other Notes, Files, Tasks, or Events. Values are arrays ofmemry://<kind>/<id>URIs, so renaming a target requires zero writes to referencing notes — display titles resolve live from the index.Steps 1 and 2 of the design (
docs/superpowers/specs/2026-08-03-relational-properties-design.md). Dot-path references ({{john_doe.father.email}}) and rollups/query views are specified there but not built here; rollups are gated on definition sync.Why
Beta feedback, 2026-07-22:
What you get
Design decisions worth reviewing
propertiesrecord. No newSyncItemType, no data-DB schema change. The only migration is0020, index-DB only, purely additive (CREATE TABLE+CREATE INDEX).property_refsis index-DB only and rebuildable, likenote_links. Populated insidesetNoteProperties, the single choke point both the indexer and the sync-pull path already call.PropertyDefinitionSchemais untouched.relationcarries no config, so it followstext/number/url. This keeps.memry/properties.mdformat-stable — important, because an unparseable definitions file makes the loader discard every definition in it.memry://URIs is a relation regardless of what the definition row says. This is load-bearing: a new relation property's first write is an empty[], which inferstextand would otherwise pin the definition totextforever, flattening the array to a JSON string in the vault file on the next property edit. Applied on both the projector path and the sync-update path.Verification
Green:
lint,typecheck(except pre-existingschemas.tsTS1117, confirmed identical at merge base), 12824 tests,check:architecture,check:contracts,ipc:check,docs:impact --strict,docs:build.Runtime-verified: two-device round trip through a real two-Electron-process harness against a real sync-server — the second device resolved chips with zero definition sync, proving the inference path. Index rebuild verified: deleting the index DB repopulates
property_refs, and backlinks and graph edges return.Not verified — worth doing before merge: the GUI click-through. Add a Relation property, pick a target, reopen the note, then edit a different property, and confirm the chips,
property_refsrows, and graph edge all survive. The Critical bug above lived exactly in that sequence; it is now covered by seam tests, but not by a real UI run.Known limitation, not introduced here
On a client that has typed a relation property as
text, any push of that note ships the value as a flattened JSON string — a body edit, tag change, or rename is enough, since both push builders serialize properties from the index DB rather than the file. This is a pre-existing class rather than something relation introduces: property definitions never sync, so an existingmultiselectarray created on one device flattens identically on another. The spec documents it accurately. The real fix is that the renderer rebuilds the whole property record from a lossy cache, and fixing that closesmultiselectat the same time.