Derive a Subject's name from facts instead of carrying it - #1362
Closed
JeroenDeDauw wants to merge 1 commit into
Closed
Derive a Subject's name from facts instead of carrying it#1362JeroenDeDauw wants to merge 1 commit into
JeroenDeDauw wants to merge 1 commit into
Conversation
Follows-up to #1309 and #1359. `displayName` and `displayNameIsGenerated` on the frontend `Subject` were one fact in three fields, with an invariant only `withLabel` maintained. Clearing a label kept the removed label as the name, so the save toast announced a name the Subject had just lost. The entity now holds stored facts only: id, nullable label, Schema, statements. `SubjectWithContext` gains `isMainSubject` and derives the chosen name from label, page name and that flag; the presentation layer adds the Schema fallback and the `(unnamed …)` marker. The rule exists once in PHP (`SubjectDisplayName`) and once in TypeScript (`chosenSubjectName`), cross-referenced and each unit-tested on the same four cases; no shared fixture ties them. Deriving in the browser is also what multilingual labels will need: which label to show depends on the reader's language, which the browser knows and the server does not. A server-computed name would have needed a language parameter on every read. REST returns `isMainSubject` for every Subject and drops `displayNameIsGenerated`; a GitHub code search finds no consumer outside this repository. `displayName` stays for clients that would rather not derive it. Lua, `{{#neowiki_value}}`, RDF and the graph are unchanged. Cost: every Subject in a response now says whether it is its Page's Main Subject. Only the Page knows, so the two read queries (a Subject with its relation targets; a Page's Subjects with their referenced Subjects) read each Page that has a Subject in the response, once per Page, where a label used to skip that read. The Main Subject flag on the subject-page index (#1335) would make it an index read. Not this PR's question: whether the Page-name fallback should exist at all. That is ADR 31's. Dropping it later is a one-line change in the PHP rule and its TypeScript mirror, and would remove the read above. Visible change: after clearing a label, the toast and the tree name the Subject `(unnamed <Schema>)`. #1356 needs a rebase: eight shared files and three `getDisplayName()` calls, which become `subjectDisplayName( subject )`. Considered, omitted: - Renaming PHP `SubjectDisplayName`, which decides what the graph and RDF materialize, not what is displayed. - Dropping the Schema tier from `rdfs:label`, which puts into the data the ambiguity PR 1359 removed from the UI. - A name-source enum on the wire instead of `isMainSubject`. - Memoizing page content reads in the repository to absorb the extra read: the save path mutates the loaded `SubjectContent` in place, so a memo needs invalidation; a change of its own if the cost shows in measurements. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
JeroenDeDauw
force-pushed
the
subject-name-from-facts
branch
from
September 7, 2026 17:36
cc2132d to
3de1526
Compare
Member
Author
|
Started a deeper investigation into the sensibility of having "domain/" in the FE code. Some things seem off. The goal I started pursuing with this PR might not be the right one. |
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.
I got triggered by the new
displayNameanddisplayNameIsGeneratedfields in TSSubject. Display concerns leaking into the domain model. This PR fixes that.Follows-up to #1309 and #1359.
displayNameanddisplayNameIsGeneratedon the frontendSubjectwere one fact in three fields, with an invariant onlywithLabelmaintained. Clearing a label kept the removed label as the name, so the save toast announced a name the Subject had just lost.The entity now holds stored facts only: id, nullable label, Schema, statements.
SubjectWithContextgainsisMainSubjectand derives the chosen name from label, page name and that flag; the presentation layer adds the Schema fallback and the(unnamed …)marker. The rule exists once in PHP (SubjectDisplayName) and once in TypeScript (chosenSubjectName), cross-referenced and each unit-tested on the same four cases; no shared fixture ties them.Deriving in the browser is also what multilingual labels will need: which label to show depends on the reader's language, which the browser knows and the server does not. A server-computed name would have needed a language parameter on every read.
REST returns
isMainSubjectfor every Subject and dropsdisplayNameIsGenerated; a GitHub code search finds no consumer outside this repository.displayNamestays for clients that would rather not derive it. Lua,{{#neowiki_value}}, RDF and the graph are unchanged.Cost: every Subject in a response now says whether it is its Page's Main Subject. Only the Page knows, so the two read queries (a Subject with its relation targets; a Page's Subjects with their referenced Subjects) read each Page that has a Subject in the response, once per Page, where a label used to skip that read. The Main Subject flag on the subject-page index (#1335) would make it an index read.
Not this PR's question: whether the Page-name fallback should exist at all. That is ADR 31's. Dropping it later is a one-line change in the PHP rule and its TypeScript mirror, and would remove the read above.
Visible change: after clearing a label, the toast and the tree name the Subject
(unnamed <Schema>).#1356 needs a rebase: eight shared files and three
getDisplayName()calls, which becomesubjectDisplayName( subject ).Considered, omitted:
SubjectDisplayName, which decides what the graph and RDF materialize, not what is displayed.rdfs:label, which puts into the data the ambiguity PR 1359 removed from the UI.isMainSubject.SubjectContentin place, so a memo needs invalidation; a change of its own if the cost shows in measurements.Production notes
Design and this description by
Fable 5.1 (max); implementation by anOpus 5subagent from a written spec. The subagent removed the label short-circuit in the two read queries on its own, after finding it madeisMainSubjectwrong for labelled Subjects; accepted on review, and it is the source of the read cost above.