Krow/krow_talent_app
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

candidates and board ui agent issue
Some checks failed
CI / check (push) Failing after 4m58s
Details

Browse Source
This commit is contained in:
Aravind
2026-09-05 10:46:06 +05:30
parent e02a0c23d4
commit 6249e00a3a
78 changed files with 15071 additions and 1998 deletions
Download Patch File Download Diff File Expand all files Collapse all files

393
docs/instanced-ui-nodes.md Normal file
View File

@@ -0,0 +1,393 @@
# Repeated and instanced UI nodes — design
> **Status: design only. Nothing here is implemented.**
> The audit below is traced from the code as it stands; the design that follows
> is a proposal to be reviewed and tested before any of it is built.
Every page migrated so far is *flat*: each node in the composition renders
exactly once, so a node id and a rendering are the same thing. Positions is the
first surface where that is not true. Six of its eight extension points render
**inside a record** — once per position — and the card those records are drawn
in is hand-written JSX that the node system cannot see at all.
This document says what is there now, proposes a model for it, and is honest
about what the model costs.
---
## 1. What is there now
### 1.1 The eight Positions placements, and which are instanced
`surfaces.js:47-76` declares eight placements under the `positions` surface and
`provides` already records the distinction that matters — which of them hand a
section a record:
| Placement | Rendered | Host | `provides` |
|---|---|---|---|
| `after-position-list-summary` | once per page | `Positions.jsx` (grid) | `[]` |
| `after-position-list` | once per page | `Positions.jsx` (grid) | `[]` |
| `after-position-card` | **once per record** | `PositionCard` | `positionId` |
| `after-header` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
| `after-position-summary` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
| `before-candidates` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
| `after-candidates` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
| `before-footer` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
Only the first two are in the node tree today (`pages/admin/positions/nodes.js`),
drawn through `UiNodeSlot`. The other six are still literal `<SkillSurface>`
elements.
### 1.2 The render loops
**The grid** — `Positions.jsx:1158`:
```jsx
{filtered.map((p) => (
<PositionCard key={p.id} position={p} onOpen={openPosition} isRecent={recentIds.has(p.id)} />
))}
```
`filtered` is page state: the search box, the role and location selects and the
sort, applied in `useMemo`. It is not a data source in the `surfaces.js` sense
and has no entry in `DATA_SOURCES` — it is the page's own working set.
**Inside each card** — `Positions.jsx:626-641`. The card's last element, wrapped
in a click-stopping `div` so a section inside a card does not open the drawer
behind it:
```jsx
<SkillSurface page="positions" placement="after-position-card"
context={{ position: p }} className="mt-3" />
```
**The drawer** — `PositionDrawer` (`Positions.jsx:719, 729, 793, 870, 897`) draws
five surfaces, each with `context={{ position: p }}`, for the one position
`openPosition` selected. `PositionDetail.jsx:288, 421, 472, 512, 513` draws the
same five placements for the position named in the route.
### 1.3 How a record reaches a section
Unchanged all the way down, and already correct for this design:
```
<SkillSurface context={{ position: p }} />
→ useSkillDataContext(context) SkillSurface.jsx:76 { ...published, ...context, ...collections }
→ resolveSkillData(section, ctx) dataResolver.js:1192
if (section.context === 'positionId' && !context.position) → unavailable
RESOLVERS['position.pipeline']({ position, applications }) → filters by position.id
```
An explicit `context` prop beats what the page published, which is exactly the
rule an instanced node needs: *a card knows which position it is.*
### 1.4 Section resolution
`useSkillSections(page, placement)` (`SkillSurface.jsx:46`) is independent of the
record. It reads the account's active skills and returns
`{ skill, section }[]` for the placement. **The same list is used for every
card** — repetition happens below it, in data resolution, not in which sections
exist. That is what makes one template node correct.
### 1.5 DOM identity today
None inside a card. `<SkillSurface>` renders `div.space-y-4 > section[aria-label]`
with no `data-` attributes, and `SkillSection`'s React key
(`` `${skill.id}:${section.id}` ``) is not emitted. `PositionCard` renders an
`<article>` with no id. There is nothing in the document that says *which*
position a rendering belongs to.
### 1.6 The card body
`PositionCard` (`Positions.jsx:519-643`) is roughly 120 lines of tuned JSX:
role glyph and client/title/category block, status pill, a terms `<dl>`, one
line of candidate criteria, a hairline rule, `Progression`, `WorkforceRow`, a
health chip with the insight line, and a footer pinned with `mt-auto` so a row
of cards keeps its footers aligned. Plus a two-minute "just saved" treatment.
**None of it is addressable.** "Make all position cards compact" has nothing to
act on today, in either the node tree or the component: no density prop exists.
---
## 2. The model
### 2.1 One node, many renderings
The rule the whole design rests on:
> A repeated surface is **one node in the tree**. The tree holds the template
> once; the DOM holds N renderings of it.
An operation targets the node, so one operation changes every card — which is
the "do not duplicate operations once per record" requirement met in the stored
bytes, not by a de-duplication pass.
### 2.2 Repetition is declared by the type, never by a patch
```js
registerNodeType({
type: 'position-card',
label: 'Position card',
component: PositionCardNode,
container: true,
accepts: ['skill-surface', ...],
repeats: {
from: 'positions', // a key in the bag the PAGE publishes to UiRenderContext
as: 'position', // the context key each rendering is given
key: 'id', // the record field that identifies a rendering
},
propSchema: { density: { enum: ['comfortable', 'compact'] } },
capabilities: ['update', 'move', 'hide', 'reorder'],
});
```
`repeats` lives on the **registration**, which is code, and never on the node —
so it is absent from `OP_FIELDS`, cannot be written by a stored patch, and gives
the validator nothing new to police on user data. A patch can change what a card
looks like; it can never change what a card iterates over.
`from` names a key in the page's own `UiRenderContext` bag
(`UiTreeRenderer.jsx:39`) rather than a `DATA_SOURCES` entry, because `filtered`
*is* page state — search, filters and sort applied. This adds no new data
pathway: the renderer reads `useUiContext()[entry.repeats.from]` and still never
looks inside the bag on its own account.
### 2.3 Node schema — unchanged
No new field on `UiNode`. A repeater is an ordinary container node whose *type*
happens to repeat:
```js
{
id: 'position-card',
type: 'position-card',
props: { density: 'comfortable' },
layout: { ... },
children: [ /* the template subtree */ ],
hidden: false, origin: 'builtin', locked: false,
}
```
The template's children are ordinary nodes with ordinary ids
(`position-card-terms`, `position-card-extensions`, …). Each is one node and N
renderings, by the same rule.
### 2.4 DOM identity
```html
<article data-ui-node="position-card" data-ui-instance="pos_123"> … </article>
<article data-ui-node="position-card" data-ui-instance="pos_456"> … </article>
```
`data-ui-node` keeps meaning **the node** — one value, N elements. `data-ui-instance`
carries the record key from `repeats.key`. Descendants of a rendering inherit the
instance from their ancestor rather than repeating it, so the pair
(`data-ui-node`, nearest ancestor `data-ui-instance`) addresses exactly one
rendering.
This **breaks the current invariant that `data-ui-node` is unique in a
document**, and everything that assumes it must be found and changed. See §5.1.
---
## 3. Targeting semantics
Three scopes. Only the first is proposed for implementation now.
### 3.1 Template scope — the default
```json
{ "op": "update", "target": "position-card", "props": { "density": "compact" } }
```
No new field. Applies to the node, therefore to every rendering. This is
"make all position cards compact", and it is one operation regardless of how
many positions exist.
Everything already true stays true: the op is validated against the type's
`propSchema` and `capabilities`, refused if `density` is not a declared enum
value, and stored in `uiLayouts` like any other.
### 3.2 Instance scope — designed, not built
An optional `scope` on the **operation**, not on the node:
```json
{ "op": "update", "target": "position-card",
"scope": { "key": "pos_123" },
"props": { "density": "comfortable" } }
```
The tree stays one template. `applyPatch` partitions:
- **unscoped ops** are applied to the tree as they are today;
- **scoped ops** are attached to their target node, grouped by key, and applied
at render time to that one rendering — by `applyOperations`, the same engine,
with the same validator.
There is no second mutation engine and no per-record tree in storage.
**Legality is declared per operation, not per page.** Each entry in `OPERATIONS`
gains `instanceable: true|false`:
| Operation | Instanceable | Why |
|---|---|---|
| `update` | yes | changes one rendering |
| `hide` | yes | changes one rendering |
| `move`, `reorder` | no (first cut) | one card structurally unlike its neighbours |
| `add`, `remove`, `replace` | no (first cut) | "remove this record's card" is a filter, not a layout change |
A `scope` on a node whose type does not declare `repeats` is refused — a registry
read, not a page branch.
### 3.3 Predicate scope — reserved, not designed
`scope: { where: [...] }` — "make all *draft* position cards compact". Named here
only so `scope` is an object from the first day rather than a bare key string
that would have to be widened later.
### 3.4 Natural language
Template scope needs nothing new: `resolveTarget` already scores "the position
card" against node titles, labels and ids, and there is exactly one such node.
Instance scope needs a record resolver — "this position's card" is only
answerable when a position is selected, which `PageContext` publishes for the
drawer but not for the grid. **Deferred.** Until then Owliver refuses instance
phrasing explicitly rather than silently widening it to every card, which is the
failure worth guarding hardest: a person who says "only this one" must never get
"all of them".
---
## 4. Persistence
### 4.1 Shape
No new store, no new key, no new tier. `preferences.uiLayouts[page].ops` gains an
optional `scope` per op:
```json
{
"schema": 2,
"page": "positions",
"updatedAt": "…",
"ops": [
{ "op": "update", "target": "position-card", "props": { "density": "compact" } },
{ "op": "hide", "target": "position-card-pay", "hidden": true },
{ "op": "update", "target": "position-card",
"scope": { "key": "pos_123" }, "props": { "density": "comfortable" } }
]
}
```
Size is **O(operations), not O(records)**. One op makes every card compact.
### 4.2 The schema bump is not optional
`OP_FIELDS` (`patch.js:51`) whitelists the fields each op may carry and
`normalizeOp` **silently drops anything else**. A build that predates `scope`
would therefore read the third op above as an unscoped one and make *every* card
comfortable — the exact "only this one became all of them" failure.
So:
- a patch containing **no** scoped op keeps `schema: 1` and every existing build
reads it exactly as it does today;
- the first scoped op written raises that page's patch to `schema: 2`;
- a reader that does not understand a schema **skips the whole patch** and
reports it through the existing `skipped` channel, rather than applying part
of it.
### 4.3 Stale keys
A scoped op naming a record that no longer exists is reported as `skipped`, like
any stale target, and **retained** rather than dropped. Retained deliberately:
the record may simply be filtered out of `filtered` by the page's own search or
role filter at that moment, and treating the page's filter state as deletion
would quietly destroy a person's saved overrides every time they typed in a
search box. Clearing them is an explicit action in the editor.
---
## 5. Risks
**5.1 `data-ui-node` stops being unique.** Any `querySelector` on it silently
takes the first rendering — in tests, in the editor, and in anything built later.
*Mitigation:* replace the uniqueness assertion with the real invariant — ids are
unique unless the node's type declares `repeats`, and (`data-ui-node`, ancestor
`data-ui-instance`) is unique always. This is a test to write **before** the
first repeater exists.
**5.2 Render cost.** Instance-scoped ops run the operation engine per rendering.
Negligible for a handful of overrides over tens of cards; not negligible for
hundreds of records. *Mitigation:* only records that actually carry a scoped op
do any work, memoized on (template, key, ops hash).
**5.3 Migrating `PositionCard` is a real rewrite, and the riskiest one so far.**
`mt-auto` footer alignment, truncation, the hover and focus rings, and the
"just saved" animation are all tuned. Phase 5A already produced one margin
regression on a far simpler surface. *Mitigation:* the same baseline capture
used for every other migration, run over the grid with a fixed record set, plus
the class-signature and word-signature checks.
**5.4 "Compact" does not exist yet.** No component honours a density prop. The
engine cannot invent one: the schema is what makes a change *expressible*, not
what makes it *possible*. Someone must implement two densities in `PositionCard`
before "make all position cards compact" can do anything, and until then the
honest answer to that request is that the card offers no such setting.
**5.5 The drawer and the detail page draw the same five placements.** They are
never on screen together — the drawer overlays `/admin/positions`, the detail
page is `/admin/positions/:id` — but they are different layouts around identical
extension points. If both render one composition, a change made in the drawer
also changes the detail page, which may surprise. *Proposal:* one composition
(`position-detail`) drawn by both hosts, editable from the detail route only in
the first cut, because an editing session currently holds a single composition
keyed to the route. Making a session span two compositions is the extension, and
`uiLayouts` already stores per page so it needs no storage change.
**5.6 Per-record overrides invite chaos.** Twenty individually tweaked cards is
unreviewable. *Mitigation:* keep instance scope to `update` and `hide`, show the
count of overrides on the node in the editor, and offer to clear them all.
**5.7 `repeats.from` is a page-state key, so a rename fails at run time.** The
page renames its context key, the repeater finds nothing, the grid renders empty
— with no build error. *Mitigation:* a check-script assertion that every
registered `repeats.from` is a key the owning page publishes, in the same shape
as the surface-route guard.
**5.8 Skill sections inside a repeater lose id uniqueness too.** A section under
`after-position-card` is one node and N renderings, each resolving against a
different `position`. Consistent with the model, and `SkillSurface`'s contract is
unchanged — `as: 'position'` feeds exactly the `context={{ position }}` prop it
already takes — but it is the same uniqueness caveat as §5.1 reaching the skill
layer.
**5.9 The MD schema is untouched.** No frontmatter key, no `normalizeSection`
change, therefore no Go parser change and no oracle regeneration. Confirmed by
construction: nothing in this design is authored in Markdown.
---
## 6. What must be tested before any of it is built
Against the *existing* Positions architecture, so the semantics are proven
before the card is touched:
1. One `update` on a repeater node changes every rendering — asserted on
rendering count, not on one element.
2. That change is **one** operation in `uiLayouts`, with a record count > 1.
3. A scoped `update` changes exactly one rendering and leaves the others.
4. A scoped op on a non-repeating node is refused.
5. A `move`/`remove`/`add`/`replace` carrying a `scope` is refused.
6. A schema-2 patch read by a schema-1 reader is skipped whole, never widened.
7. A scoped op naming an absent record is reported skipped and **retained**.
8. Filtering the grid does not drop overrides for the filtered-out records.
9. `data-ui-node` × ancestor `data-ui-instance` is unique; ids repeat only for
types declaring `repeats`.
10. Owliver refuses instance phrasing rather than widening it to the template.
11. A skill section under `after-position-card` resolves against its own card's
position, in every rendering.
12. The Positions grid renders identically to its captured baseline.