feat(viewer): render a typed entity page's declared fields and link formats - #189
feat(viewer): render a typed entity page's declared fields and link formats#189PipDscvr wants to merge 4 commits into
Conversation
Slice A Task 1 of atomicstrata#178. `profilePipeline` now carries `titleField`, the declared `fields`, and the profile's `artifactTypes` — the read model §1 asks for, extending the block atomicstrata#177 introduced rather than adding a parallel `profileSchema`. Four of §1's six items already shipped there, so a second block would have put the lifecycle on the wire twice and given a reader two sources that can disagree about it. The drop boundary is closed by the type system, not by a test. The facets the wire refuses — `default`, which can carry arbitrary author-supplied data, and `min`/`max`, which nothing renders — are named as `DroppedFieldFacet`, and everything else is an EXHAUSTIVE `Record<ProjectedFieldFacet, true>`. A facet added to `FieldDef` fails to compile there until someone decides which side it belongs on; an array would have silently accepted a subset. Verified by probe: adding a field to `FieldDef` produces TS2741 at that record. Fields ride as an ordered array rather than a map, because object key order is not a contract a client should trust and the author's declaration order is the only order this projection does not invent. `maxBytes` is deliberately not projected — it is a ceiling enforced on the handle, not something a reader is shown. It belongs on the wire the day a surface renders it. The envelope join is tested separately from the projection: an artifact type has no count to join onto it, so it could be dropped there without any projection test noticing, and the shipped newsroom fixture declares no artifact types to catch it.
Slice A of atomicstrata#178: §1 the read model, §2 generic rendering, §6 the field-format vocabulary, and the AutoSci configuration those need. **The read model extends `profilePipeline`** rather than adding a parallel `profileSchema`, per the issue thread: four of §1's six items already ship there, and a second block would put the lifecycle on the wire twice. It gains `titleField`, the declared `fields`, and a sibling `artifactTypes`. The projection's drop boundary is closed by the type system. The refused facets — `default`, which can carry arbitrary author-supplied data, and `min`/`max`, which nothing renders — are named as `DroppedFieldFacet`, and the rest is an exhaustive `Record<ProjectedFieldFacet, true>`. Adding `format` to `FieldDef` in this same change failed to compile there until it was explicitly projected, which is the guard doing its job rather than a hypothetical. **The renderer dispatches on declared type, never on a field name.** A renderer that knew what `doi` meant would work for one profile and quietly do nothing for the next, and `src/` may not name a domain vocabulary at all. `no-research-branch-in-core` stays green and unedited. Three value states are reachable and no more: present, absent-and-optional, and unresolved artifact. A record violating its field contract never becomes a viewer page, so there is no fourth branch to write. **The rail's own list was the default profile's vocabulary**, not raw frontmatter as assumed — nine fixed keys describing a contract a typed page was never under. So nothing disappears: the declared fields lead, and that list still runs, minus any key the profile declares and minus the field the heading already shows. **`format` is a closed enum with fixed origins.** A URL template would be executable profile behaviour on a read surface. The renderer guards independently of load validation — `/api/pages` is a wire boundary — and returns text for anything it is not certain about, so a `javascript:` value renders as text rather than as a link built anyway. AutoSci declares `doi`/`arxiv`/`url` formats and advances to 0.3.0, with 0.1.0 and 0.2.0 both retained and their published digests pinned. Review findings addressed: the `format` rule now covers all three `FieldDef` carriers (relation attributes and artifact metadata resolve to the same `$defs/fieldDef`, as `assertArtifactTypesScoped` already accounts for); `artifactRef[]` no longer renders as plain scalars, which would have shown unverified refs indistinguishably from checked ones; the label rule is scoped to out-specify `.support-rail dt`, which was silently winning; and the page route awaits the envelope so both render passes paint the same rail. One reported finding — an `empty-page` lint regression — was refuted: the linter reads `meta.title` directly and never sees the collector's resolved title.
Self-audit findings that belong to Slice A. The two title-related regressions the same audit found are fixed in the parent PR, since that is the PR that introduces them. **Nothing bound the projection to the renderer.** Both halves were pinned against a hand-written envelope in the middle, so they agreed by care rather than by construction — a fixture drifting in lockstep with its consumer would have passed both suites with the feature dead in the browser. A new contract test runs the real projection and feeds its output to the real renderer; breaking the seam now fails it. **The DOI grammar rejected valid DOIs for no security gain.** It forbade `/`, `?` and `#` in a suffix to stop path steering — but the origin is a literal prefix, so no suffix can escape it. It just dropped real identifiers like `10.5061/dryad.abc/1` to plain text. The grammar now identifies and a new origin check contains, which is the property that actually mattered. Returning the resolved `href` also means the validated string is the navigated one. **A `titleField` naming a fixed-rail key resurrected in the other list.** Hiding it from the declared block stopped it suppressing the fixed row, so it reappeared beside the heading it duplicates. Suppression is now computed from the unfiltered declarations. **The cascade was untestable.** The JSDOM suites assert DOM structure and never load a stylesheet, so `.entity-field-label` losing to `.support-rail dt` was invisible to every test — review caught it, nothing else could. A specificity guard now covers that failure mode, verified by reintroducing the exact bug that shipped and watching it fail. It checks specificity and source order only, which is what that bug was; it is not a browser, and its docblock says so. Also: the new link colour joins the contrast pins, where it clears 4.5:1 in both themes.
Same class of bug as the `titleField` lookup corrected in atomicstrata#187, in the two places slice A introduced it. The profile schema constrains a field's `type` to a closed enum but puts no `propertyNames` on `fields` or on the entity map, so a declared NAME is the open half — and every lookup below indexes a plain JSON object with one. `viewer-entity-fields.js` read `frontmatter[def.name]`. Frontmatter arrives as JSON off the wire, carrying `Object.prototype`, so a type declaring a field named `constructor` resolved `Object.prototype.constructor` on EVERY page of that type: `isPresent` saw a function, and the row rendered `function Object() { [native code] }` as the field's value — a value the record does not carry, displayed as though it does, which is the one thing this surface exists not to do. `toString` and `valueOf` behave the same. `FIELD_RENDERERS` already took the null-prototype form of this precaution for the declared type; this is the same precaution for the declared name. `pipeline.ts` joined counts onto declarations by indexing summary maps with a declared type name. A type named `constructor` resolved a function, which then read as a truthy `stateCounts` and as a `count`/`pageCount` that `JSON.stringify` drops — so the row reached the client missing a key it is required to carry. The lifecycle `enum` lookup had the same shape; that one resolved to `undefined` either way, so it is defence in depth rather than an observable fix, and shares the helper rather than being left as the one bare index in the file. Both are pinned by tests that fail without the fix — the renderer case renders "native code" into the page, and the envelope case asserts `expected [Function Object] to be +0`. The guard matches the `Object.hasOwn` convention already used in `validate.ts` and `validate-workflow-actions.ts`.
|
Reviewing this as a draft, because early signal on 2000 lines is worth more than late signal. One thing to fix; everything else here is confirmation. You honoured every ruling from #178, including the one where I overruled you: One thing to fix: reserved characters truncate the DOI before it reaches the resolver
This is reachable rather than theoretical, since the Crossref connector preserves the DOI body as written. The fix is to encode the reserved characters in the suffix while leaving the DOI path separators alone. Worth pinning Crossref's published example as a regression test, and Nothing else in the guard moves. The origin comparison is doing the real containment, exactly as your docstring argues, and it holds under everything I threw at it: Two things worth naming because they were not asked forThe regression check is stronger than the one I specified. I asked that nothing the rail shows today disappears. What you wrote drives the real projection into the real renderer, and opens with You swept Checked rather than readThe retained-release digests survived the rename to Both guards kill their mutants: passing everything through the link formatter turns 14 tests red, and letting format validation accept any field type turns 4 red. It merges clean onto current Good work. Fix the DOI encoding and I think this is done. |
2 participants
Slice A of #178, per the A/B/C split agreed on that issue. Refs #178 — it does not close it; B (relations, provenance) and C (artifacts, the two source surfaces) follow.
A typed entity page used to show its body beside a rail describing a contract it was never under: the rail's nine fixed keys are the default profile's vocabulary, so a paper's authors, year, DOI and stage were simply invisible. This makes the page render what its own type declares.
What's in it
§1 — the read model.
profilePipelinegainstitleFieldandfields, plus a siblingartifactTypes. NoprofileSchemablock:type,directory,lifecycleand the relation endpoints already ship inprofilePipeline, and a second block able to disagree with it about a lifecycle is the shape we've spent three PRs removing. The name is kept, with a docstring stating the block's remit.§2 — generic rendering. The renderer dispatches on the declared
FieldDef.typeand never on a field name;test/no-research-branch-in-core.test.tsstays green and unedited. Three value states are reachable and no more — present, absent-and-optional (row omitted), and unresolved artifact reference — because a record violating its declared field contract never becomes a viewer page:collectTypedViewerInputsfilters it and it surfaces as afield-violationproblem instead. There's a comment at the branch point saying why there is no fourth state.§6 — declarative link formats. A closed
url | doi | arxivenum, no author-supplied URL templates. Both layers: an unknown format fails validation, and the renderer falls back to plain text for anything unknown or malformed rather than trusting that a validator ran upstream. Origins are out of author control entirely —urllinks onlyhttp:/https:,doithrough a fixedhttps://doi.org/,arxivthrough a fixedhttps://arxiv.org/abs/.AutoSci declares
doi,arxivIdandlocatorformats, taking it to0.3.0.0.1.0and0.2.0are both retained as resolvable releases, derived backwards from the current pack —0.2.0is0.3.0without the formats,0.1.0is that without the title declarations — with each publishedprofileDigestpinned intest/profile-template-releases.test.ts, so a later edit that corrupts a derivation fails there rather than quietly mis-describing an installed project.Digest drift, and what it is not
Adding
formatto aFieldDefshifts the profile and template digests. It does not reclassify in-flight workflow runs:classifyRuncomparesworkflowDefDigest(def)againstrun.workflowDigest(src/workflows/status.ts:181-189), andrun.profileDigestis pinned at start and shape-checked inrun-validate.ts:101but read by no classification path. Calling this workflow adaptation would send people looking for an effect that isn't there.Nothing disappears from the rail
Checked rather than assumed, since it was the one open question on §2.
appendFrontmatterDliterates a fixed nine-keyRAIL_FIELDSlist and skips a key only when the declared block already rendered it. The rail never displayed arbitrary frontmatter, so a key that is both declared and in the nine moves between lists rather than being lost, and an undeclared key outside the nine was never displayed before and still isn't. No regression to place.One bug found reviewing my own work
bfd45f9is the same class of defect as thetitleFieldlookup corrected in #187, in the two places this slice introduced it. The schema constrains a field'stypeto a closed enum but puts nopropertyNamesonfieldsor on the entity map — so the declared name is the open half, and every one of these lookups indexed a plain JSON object with one.viewer-entity-fields.jsreadfrontmatter[def.name]. Frontmatter arrives as JSON off the wire, carryingObject.prototype, so a type declaring a field namedconstructorresolvedObject.prototype.constructoron every page of that type and renderedfunction Object() { [native code] }as the field's value — a value the record does not carry, displayed as though it does.toStringandvalueOfbehave the same.pipeline.tsjoined counts onto declarations by indexing summary maps with a declared type name, so a type namedconstructorproducedexpected [Function Object] to be +0— andJSON.stringifythen drops the key, so the row reached the client missing apageCountit is required to carry.Both are pinned by tests that fail without the fix.
FIELD_RENDERERSalready took the null-prototype form of this precaution for the declared type; this is the same precaution for the declared name.Worth your call separately: I hardened the consumers, which is this slice's remit. The underlying gap is that the schema puts no
propertyNamesconstraint on declared names at all. Constraining them upstream would touch profile validation and could reject profiles that currently load, so I didn't fold it in — but it's the root, and it will keep producing this bug class.Testing
To see it:
llmwiki viewon an AutoSci project and open a paper — authors, year, stage and a working DOI link where there was a slug and a generic rail. The same renderer against the Newsroom fixture, with no domain-specific conditional.Full gate:
tsc --noEmit,npm run build, 4989 passed / 0 failed,fallow0 above threshold with no dead code or duplication.One environment note: the suite needs
LLMWIKI_PROVIDER=anthropicwith noANTHROPIC_API_KEYon a machine where Claude Code is logged in —claude-agentrequires no credential env var (provider-guard.ts:45) and authenticates from~/.claude, so tests asserting a missing-credential error find working auth and fail. Same note as on #179.Next
B — §3 relation drilldown and §4 provenance, byte-free, reusing the relation instances already captured at snapshot creation, with the per-page list capped and the true total reported separately following
/api/reviews. §4 links to the source entry, not the cited lines.C — §5 artifacts and §7 the two source surfaces, written last so they sit at the top of the log: bytes on loopback only, non-loopback gets metadata and health, and the pinned hash re-verified at request time on every artifact response.