Skip to content

Find a relation's target Subject by its id - #1357

Merged
JeroenDeDauw merged 1 commit into
masterfrom
1326-find-target-by-id
Sep 6, 2026
Merged

Find a relation's target Subject by its id#1357
JeroenDeDauw merged 1 commit into
masterfrom
1326-find-target-by-id

Conversation

@alistair3149

@alistair3149 alistair3149 commented Sep 4, 2026

Copy link
Copy Markdown
Member

Fixes #1326

The relation picker searched Subject labels only, which left a Subject with no label unreachable:
its graph node carries no name for the label search to match, as
ADR 31
records, while Manage Subjects offers its id behind a copy button with nowhere to paste it.

Text that is a well-formed Subject id is now read as one, and the Subject it names becomes the
field's one candidate when its Schema is the one the relation targets. The picker already turned an
id into a named entry to label its own selection, so the read, its permission gate and the server's
display-name fallback all come along unchanged — including on a wiki with no graph store, where the
label search finds nothing at all.

The read adds to the label search rather than replacing it. An id is fifteen base58 characters,
which is also the shape of an ordinary word — straightforward and standardization both pass
SubjectId.isValid — so text that names no usable Subject goes on to be searched for as a label:
an id no Subject has, one the user may not read, one of another Schema, or a word that merely looks
like an id. Only a read that lands on a usable Subject ends there, so a real id still costs one
request and no search.

Both routes now see the same trimmed text. A pasted name kept its padding and so matched nothing,
while the create option beside it offered the trimmed one.

Clearing the field abandons a lookup still in flight, which could otherwise land and fill the menu
of a field the user had just emptied. Only a lookup: the sequence it bumps is shared with the
creation flow, which clearing the field is no reason to cancel.

The placeholder now says so.

Manual Browser Check

First give the wiki a Subject the label search cannot reach. On the demo data, page 40 is
ACME Engineering; the response carries the new subjectId:

curl -b cookies.txt -X POST "$WIKI/w/rest.php/neowiki/v0/page/40/childSubjects" \
  -H 'Content-Type: application/json' -H "X-Csrf-Token: $CSRF" \
  -d '{"schema":"Department","statements":{}}'

Then open ACME Inc and press Edit on the infobox.

  1. Every empty picker reads Search by name or subject ID.
  2. In an empty Departments row, type Department. The menu says "No subjects found" — that is
    the bug.
  3. Paste the subjectId. The menu offers Department, the display name the server derives for a
    Subject nobody named. Pick it: the row fills with that name and a new empty row appears.
  4. Paste s1demo1aaaaaaa2 (Acme Anvil, a Product), then sZZZZZZZZZZZZZZ. Neither is offered as a
    candidate; each falls through to a label search, which finds nothing, so the menu says "No
    subjects found".
  5. Give some Subject of the target Schema the label straightforward — fifteen base58 characters,
    so it passes as an id — and type it. It is found by name. Before the fallback it was not.
  6. Type ACME Eng with the padding. It finds ACME Engineering; before, padding matched
    nothing while the create option beside it read Create "ACME Eng".
  7. With DevTools open on the Network tab: step 2 issues one subject-labels request and no read;
    step 3 issues one subject/{id} read and no search; steps 4 and 5 issue a read that misses and
    then a search; step 6 searches search=ACME+Eng, without the padding.

Discard the dialog rather than saving, and remove the Subject again with
DELETE /neowiki/v0/subject/{subjectId}.

Testing

Eleven vitest cases cover the new behaviour, eight beside the existing picker tests and three in
the creation-enabled describe, which is the configuration the Subject editor actually uses. Four
fail against the unchanged component; the rest are negative cases that pass trivially without it,
so each was mutation-verified. Eleven mutations, all caught: removing the id branch, naming the row
by its id instead of the display name, dropping the trim, dropping and inverting the Schema check,
letting the read escape instead of answering, dropping the sequence guard, dropping the in-flight
abandon, widening that abandon back over the creation flow, and restoring the short-circuit so an
id-shaped word never reaches the label search — each broke exactly the test named after that
behaviour.

Full suite: 1849 vitest tests pass, lint and build clean. Verified in the browser on a dev wiki by
the steps above, against a rebuilt bundle with the ResourceLoader module store cleared; the network
log confirms one read for a real id, a read-then-search for an id-shaped word, and the trimmed
search.

Considered, omitted

A disabled menu entry naming both Schemas when a pasted id belongs to a Subject of the wrong
one. The generic "No subjects found" costs no code at all, while the entry would have added a
sentinel value, its guard in the selection handler, a message in three files, and tests — for a row
the user still cannot act on.

Finding a draft by its id — a Subject invented in the current editor session and not yet saved.
resolveName consults the drafts before reading the server and this path does not, so a draft's id
resolves to nothing. Left alone because a draft's id is surfaced nowhere a user could copy it from:
it exists only in a DOM attribute and an internal pane key.

A slimmer read for the id. getOrFetchSubject fetches ?expand=page|relations and uses two
fields of it. A dedicated read would be cheaper but would forfeit the registry entry that makes the
follow-up resolveName free when the user picks the item.

Related

#1313 wants one suggestion
implementation across every backend configuration, so that Neo4jSubjectLabelLookup and
NullSubjectLabelLookup can both be deleted. It was blocked on #1283, which has landed. Whoever
takes it should fold this id read into that path rather than leave two finders; the id read is an
exact lookup rather than a suggestion, so it may survive as its own step.

Known, not caused by this PR

Replacing an existing relation target via Create "X" as a new Y creates the Subject but leaves
the relation on its old target, with the field blank
(#1358). Found while reviewing this
branch and reproduced on master at 51fd2ec7, so it predates the change; the unit tests cannot
see it because the spec stubs CdxLookup with an inert component that lacks Codex's own watchers.

AI-authored — Claude Code, Opus 5 (1M context) (xhigh); issue by @JeroenDeDauw, relayed by @alistair3149, who rejected the wrong-Schema entry I had recommended and approved the placeholder change; diff not yet human-reviewed, though it has been through /code-review, /security-review, /review-tests, a design pass and a four-angle cleanup, all in the authoring session — so a fresh-context human or agent review is still worth having; 1848 vitest tests, lint and build green locally, nine mutations caught, and the flow exercised end to end in a browser against a dev wiki with the network log checked.

Production notes

The first implementation ran the id read and the label search concurrently and merged the results in the menu. The cleanup pass showed the two are mutually exclusive — the label search cannot match a well-formed id unless a Subject is named after one — so the id branch now returns early. That removed a second piece of component state, both edits to menuItems, and a test whose only failure mode was the two halves disagreeing.

One review finding was dropped on measurement rather than judgement: an assertion that an unknown id leaves the menu empty stopped discriminating once the merge landed, because a read that throws also leaves it empty. It moved to the creation-enabled picker, where a finished-but-empty search has a visible entry of its own.

@alistair3149
alistair3149 force-pushed the 1326-find-target-by-id branch from 0531d20 to 6995f45 Compare September 4, 2026 18:08
@alistair3149

Copy link
Copy Markdown
Member Author
image image

@alistair3149
alistair3149 force-pushed the 1326-find-target-by-id branch from 6995f45 to 1bb368b Compare September 4, 2026 18:40
@alistair3149
alistair3149 marked this pull request as ready for review September 4, 2026 18:45
Fixes #1326

The relation picker searched Subject labels only, which left a Subject with no label
unreachable: its graph node carries no `name` for the label search to match, as ADR 31
records, while Manage Subjects offers its id behind a copy button with nowhere to paste it.

Text that is a well-formed Subject id is now read as one, and the Subject it names becomes
the field's one candidate when its Schema is the one the relation targets. The picker
already turned an id into a named entry to label its own selection, so the read, its
permission gate and the server's display-name fallback all come along unchanged —
including on a wiki with no graph store, where the label search finds nothing at all.

The read adds to the label search rather than replacing it. An id is fifteen base58
characters, which is also the shape of an ordinary word: `straightforward` and
`standardization` both pass, so text that names no usable Subject — an id no Subject has,
one the user may not read, one of another Schema, or a word that only looks like an id —
goes on to be searched for as a label. Only a read that lands on a usable Subject ends
there.

Both routes see the same trimmed text. A pasted name kept its padding and so matched
nothing, while the create option beside it offered the trimmed one.

Clearing the field abandons a lookup still in flight, which could otherwise land and fill
the menu of a field the user had just emptied. Only a lookup: the sequence it bumps is
shared with the creation flow, which clearing the field is no reason to cancel.

The placeholder now says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Px3kMSRPQWfxr4B2qkbRoU
@alistair3149
alistair3149 force-pushed the 1326-find-target-by-id branch from 1bb368b to 3388338 Compare September 4, 2026 19:00
@JeroenDeDauw
JeroenDeDauw merged commit 10dedca into master Sep 6, 2026
20 checks passed
@JeroenDeDauw
JeroenDeDauw deleted the 1326-find-target-by-id branch September 6, 2026 18:41
JeroenDeDauw added a commit that referenced this pull request Sep 6, 2026
#1357 reached master with a five-argument
Subject constructor call after #1359 had
added the sixth, so master's TypeScript CI and Docker image build fail on
tests/components/common/SubjectPicker.spec.ts.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.

Relation picker UI should find target subjects by ID

2 participants