[malibio]

Closes #3521

Person header ("Untitled" on first open)

Cause: BaseNodeViewer's header derives from schemaFormLoader.hasTitleTemplate on its first render, before loadForm runs, because loadForm waits for the children to load.

• person declares its template on its plugin, so the loader's currentNodeType is the only input that changes when the form loads.
• currentNodeType was a plain field, so the header kept "Untitled" until something else changed the node.
• Out-of-band backend writes don't reach the frontend store (identity after onboarding, person viewer title, playbook-installed types) #3257 fixed a stale-store cause of the same symptom. This one is a reactivity gap.

Fix: currentNodeType is now $state. The node-display-title.ts comment that said no core type ships a template is corrected.

Tests (each confirmed failing with the field made plain again):

• schema-form-loader.svelte.test.ts: an effect tracking hasTitleTemplate sees it flip on loadForm('person').
• base-node-viewer-title-template-header.test.ts: mounts the viewer on a persisted person and asserts the header shows its stored title, read-only. A text page still shows its content.

TabSystem "Invalid drop operation: missing required state"

Cause: @thisux/sveltednd 0.0.20 starts a drag on every pointerdown on a tab and calls the droppable's onDrop on pointerup. The target container is only set on pointerover, so a plain click arrives as a drop with no target and logged the warning on every tab click.

Fix: readTabDrop (utils/tab-drop.ts, unit-tested) treats that case as a click. Incomplete or unparseable drop state still warns. Real moves are unchanged.

Checks

• bun run test:changed: frontend (Happy-DOM) and browser passed
• bun run quality:fix: clean

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review: PR #3523

Review Type: Initial Review (there were no earlier reviews or comments)

Summary

This is a small, focused fix that clearly improves the code. Making SchemaFormLoader.currentNodeType a $state field fixes the bug where it was not reactive. It follows ADR-049: the store class holds real reactive state, and no $effect copies state around. The tab-drop change extracts a pure, tested classifier. Before it, every plain tab click logged a warning; now a click is not logged. I checked the library behaviour this relies on against @thisux/sveltednd 0.0.20.

• The draggable's pointerdown sets isDragging, draggedItem and sourceContainer, and clears targetContainer.
• The droppable's pointerup calls onDrop while isDragging is true. Only pointerover sets the target.
• On the tab, use:droppable comes before use:draggable, so the droppable's pointerup listener runs before the draggable resets the state.

Real drag paths behave as before:

• Native HTML5 drop: dragenter sets targetContainer, so the drop is read as drop.
• Pointer path where pointerover has set the target: read as drop, and a same-index drop is still skipped.
• Drop from a foreign source with a non-tab-N container: still warns.

The three new test files pass locally (20 tests).

Requirements Check (#3521)

• ✅ A persisted person shows its stored title in the header on first open, read-only. hasTitleTemplate is now reactive through currentNodeType, and the readonly, oninput and placeholder bindings follow it.
• ✅ Regression test that mounts the viewer: base-node-viewer-title-template-header.test.ts. There is also an effect-tracking unit test in schema-form-loader.svelte.test.ts. The author confirmed both fail without the fix.
• ✅ Types without a template still show content in the header, and the header stays editable. This is covered by the second viewer test.
• ✅ The node-display-title.ts comment no longer says that no core type ships a template.
• ✅ A tab click no longer logs "Invalid drop operation: missing required state". utils/tab-drop.ts is unit-tested, and incomplete or unparseable drop state still warns.
• ✅ quality:fix and test:changed: the author reports both clean. I re-ran the new test files and they pass.

Code Review Findings

🟢 Nit: 📁 packages/desktop-app/src/lib/components/layout/tab-system.svelte:196 and packages/desktop-app/src/lib/utils/tab-drop.ts:17
readTabDrop takes draggedItem: unknown, so its drop result can't narrow the caller's draggedItem. handleDrop therefore checks !draggedItem a second time, and the 'missing-state' fallback in the reason ternary can never run. Make the helper generic (readTabDrop<T>(state: { draggedItem: T | null; ... })) and return draggedItem: T in the drop variant. The caller then destructures from drop alone, and the classifier is the only place that decides what a valid drop is. Principle: DRY, one source of truth for the validity check.

🟢 Nit: 📁 packages/desktop-app/src/lib/design/components/schema-form-loader.svelte.ts:112-114
The existing comment in loadForm says the viewer "reads it on the same render pass it calls this". The new field doc at lines 59-62 says the opposite, and it is correct: the header derives before loadForm runs, which happens after the children load. Setting the field synchronously still makes sense. The reason is now reactivity plus not waiting on the lazy import, not "same render pass". Reword the comment so the two don't disagree. Principle: comments must explain the real reason, since a stale one misleads the next reader.

🟢 Note (no action): Moving the unparseable-index log from log.error to log.warn is fine. Every container in this component is tab-${i}, so an unparseable index can only come from a drop by another source (another library user or a native drag). That is external input, not an internal invariant failing. The pane-not-found case correctly stays at log.error.

🟢 Note (no action): resetGenericSchema() and loadForm() write currentNodeType one after the other in the same synchronous block, so the header doesn't flash between the two writes. Neither write happens inside a $derived, so there's no risk of state_unsafe_mutation.

No critical or important issues. No node-type, Pro-boundary, security or performance concerns: the change is frontend reactivity and logging only.

Recommendation

APPROVE. The two nits are optional polish.

[malibio]

Review addressed

• ✅ readTabDrop single source of truth: generic readTabDrop<T>; the drop variant carries draggedItem, so handleDrop no longer re-checks it and the unreachable 'missing-state' fallback is gone. Test updated.
• ✅ loadForm comment: now gives the real reason (the header updates as soon as the type is known, not after the lazy import), consistent with the field doc.

No logic change beyond the refactor. bun run test:changed (frontend + browser) and quality:fix pass.

Re-review decision: not needed. Both changes are small, behavior-preserving refactors of review nits.