[malibio]

Closes #3440. Implements ADR-086 §8: the frontend's wire types are generated from nodespace-types, committed, and checked for drift by the merge gate.

What changed

Rust (nodespace-types)

• ts-rs is an optional dependency behind a new ts feature. Every serialized type derives TS under that feature; default builds do not compile it (cargo tree --workspace shows no ts-rs).
• A gen-ts binary (requires the feature) writes one TypeScript file per type, the core type registry (CORE_NODE_TYPES) and the typed core field table (TYPED_CORE_FIELDS, TYPED_CORE_DEFAULTS). It scans the crate's source and refuses to run if a serialized type lacks the derive or is missing from its list.
• core_promoted_fields now returns PromotedField { storage, wire, shape, read_only } instead of (storage, wire) pairs, so the frontend table can be generated from it. The core schema test pins each field's shape and read-only flag to the schema's field type and protection level.
• TYPED_CORE_DEFAULTS is computed by converting an empty node of each type, not listed by hand.

Tooling

• bun run gen:types runs the binary, formats with the frontend's Prettier config and writes packages/desktop-app/src/lib/types/generated/ (only the files that changed; a second run is a no-op).
• bun run types:check is the same with --check. It is a new stage in the merge gate (first in the Rust lane, so it fails in seconds) and in test:changed's Rust tier.
• gate-scope.ts: the generated directory, scripts/gen-types.ts and the frontend .prettierrc now reach the Rust tier, so a hand edit to a generated file runs the check.

Frontend

• The hand-written shapes in src/lib/types/ (node.ts, task-node.ts, person-node.ts, project-node.ts, query.ts, schema-node.ts, ai-chat-node.ts, core-node-types.ts, typed-core-fields.ts) and three in src/lib/services/ are replaced by re-exports from generated/. Guards, default-filling converters, primitive aliases and helpers stay.
• The two tests that regex-parsed Rust source to check parity, and their helper, are deleted; the drift check replaces them.

Drift this surfaced

• SchemaNode.description and .relationships are always serialized by Rust; the hand-written interface had description optional and no relationships. Test fixtures were updated to carry both.
• DeleteResult is deleted_count on the wire (the Rust struct has no rename_all); the frontend type said deletedCount. Nothing reads the field. The type now matches Rust; the wire shape is unchanged.
• QueryFilter.type gains related, and relationshipName / filter, which the frontend type did not have.
• TaskNode.dueDate / startedAt / completedAt are string | undefined (Rust omits them), not | null. nodeToTaskNode and setTaskDueDate now write undefined where they wrote null.
• Fields Rust types as String are string in TypeScript (AiChatMessage.role, AiChatNode.turnStatus, lifecycleStatus, nodeType). The narrower unions the frontend had (AiChatTurnStatus, …) remain as vocabulary types but no longer type those fields.

Notes for review

• NodeEnvelope.title is title?: string | null: browser mode's dev-proxy sends null for no title, while Rust omits it.
• The generated typed-node shapes inline the envelope's fields (how ts-rs renders #[serde(flatten)]), so nodeType is string, not the 'task' literal the hand-written interfaces had.
• ts-rs's no-serde-warnings feature is on: the only unsupported serde attribute in use is deserialize_with, which does not affect the shape.

Verification

• bun run test:changed (every tier, since gate files changed): frontend, scripts, skill, browser, types:check, Rust workspace all pass.
• bun run quality:fix clean.
• Adding an enum variant in Rust without regenerating makes bun run types:check fail naming the stale file; reverting makes it pass.
• scripts/gen-types.test.ts covers the drift comparison against fixture directories (changed, missing, stale, hand-edited) and that both gates run the stage.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review — PR #3471

Review Type: Initial Review (no prior reviews or comments on the PR; reviewed at e569d2dc)

Summary

This is a clear net improvement and does what ADR-086 §8 asks. The hand-mirrored wire types are gone, the generated set is committed, and both test:changed and the merge gate run the drift check. I found no blocking issue. There are three non-blocking improvements (the coverage scan fails open and is untested, the new Rust is outside clippy, and a few vocabulary types are now orphaned) and some nits.

What I verified by running or reading, rather than from the PR description:

• bun run types:check passes: "Generated TypeScript is up to date (50 files)", and the worktree stays clean afterwards.
• bun test scripts/gen-types.test.ts: 15 pass.
• svelte-check in packages/desktop-app: 2506 files, 0 errors, 0 warnings.
• cargo tree --workspace -e normal,build,dev contains no ts-rs; no crate enables the ts feature.
• Optionality, field by field: I compared every Option, Vec and bool field of every serialized struct against its serde attributes. Every field under #[ts(optional_fields)] has skip_serializing_if. The always-serialized Options (NodeReference.title, the five QueryFilter fields) come out as T | null. #[serde(skip)] on QueryFilter.resolved_relationship is honoured. Flattened envelopes are inlined with the right imports.
• Custom-Serialize enums: the generated literals for TaskStatus, TaskPriority and SchemaFieldType match their as_str() values today.

Not run: the full test suite, the merge gate, and the "add a Rust variant, watch the check fail" experiment described in the PR.

Requirements Check

	Acceptance criterion	Notes
✅	bun run gen:types writes the generated files; running it twice is a no-op	applyGenerated writes only drifted files; the check reports no drift on the committed tree. Output order is deterministic (BTreeSet/BTreeMap, CoreNodeType::ALL).
✅	Every wire type has a generated file; the frontend imports them; no hand-written interface restates a generated shape	45 type files plus 2 registries and the barrel. No hand-written interface/type in src/lib shares a generated name, except update-protocol.ts's unrelated store-level NodeUpdate, which predates this PR. See finding 3 for leftover vocabulary types.
✅	TYPED_CORE_FIELDS / TYPED_CORE_DEFAULTS are generated; typed-core-fields.ts holds no hand-maintained lists	The module is now lookups only. Defaults are computed by converting an empty node, and the values match the old hand-written table.
✅	The merge gate fails on a Rust change without regenerated TypeScript (tested with a fixture)	TIERS.typesCheck is first in the merge gate's Rust lane and runs in test:changed's Rust tier, after the machine slot is taken. The fixture tests cover the comparison (changed, missing, stale, hand-edited). The end-to-end failure was checked by hand per the PR; I did not repeat it.
✅	Default cargo builds don't compile ts-rs	Optional dependency behind ts; gen-ts has required-features = ["ts"]; confirmed with cargo tree.
✅	bun run test:changed passes; quality:fix clean	Taken from the PR for the full run. I independently confirmed types:check, the script tests and svelte-check.

Non-goal respected: no wire shape changes. PromotedField is an internal Rust API change with all three call sites updated.

Code Review Findings

Critical

None.

🟡 Important (non-blocking)

1. The coverage scan fails open in several ways and has no tests
📁 packages/nodespace-types/src/bin/gen_ts.rs:315-397

check_coverage is what backs the claim that a new wire type cannot be left without TypeScript. wire_types_in_source is a line scanner, and every one of its blind spots silently skips a type instead of failing:

• source.split("#[cfg(test)]").next() cuts the file at the first occurrence. convert.rs already has two (:559 and :1009). Nothing public sits after the first one today, but a test-only helper placed mid-file would hide every type declared below it.
• read_dir(src) is not recursive, so a type in a future src/foo/mod.rs is never scanned.
• Only Serialize counts. A Deserialize-only request payload is a wire type under ADR-086 §1 but is ignored.
• A // comment between the attributes and the pub struct line stops the attribute walk, so the derive is not seen.

The scanner also has no tests. The binary only compiles under --features ts, which the nextest run never enables, so the "missing derive" and "not listed" paths were only exercised by hand.

Practical risk is low: a type that is referenced by a listed type but not listed itself produces an unresolved import that svelte-check catches. The exposure is a brand-new top-level type.

Recommendation: strip only #[cfg(test)] followed by mod; walk src/ recursively; match Serialize or Deserialize; and move the scan somewhere a test can reach it. Since scripts/gen-types.ts already owns the tested half, the scan could live there under bun:test with fixture strings.

Principle: a guard that fails open gives false assurance; guards should fail closed and be tested on their failure paths.

2. The generator and every cfg_attr(feature = "ts", …) attribute are outside clippy
📁 package.json:84 (rust:lint), packages/nodespace-types/src/bin/gen_ts.rs

rust:lint is cargo clippy --all-targets with default features, so the 429-line gen_ts.rs (skipped via required-features) and the ts attribute paths are never linted. Their only compile is inside types:check.

Recommendation: add cargo clippy -p nodespace-types --features ts --all-targets -- -D warnings to rust:lint.

Principle: the same quality bar for all committed code ("no lint suppression" should not be bypassed by a feature gate).

3. Vocabulary types left with no consumer
📁 packages/desktop-app/src/lib/types/ai-chat-node.ts:34,43, node.ts:40, project-node.ts:17, schema-node.ts:24

Now that the generated fields are string:

• AiChatTurnStatus and AiChatSessionStatus have zero references anywhere in src.
• LifecycleStatus, ProjectStatus and the ProtectionLevel alias are only re-exported from index.ts; nothing imports them.

The PR describes them as kept "as vocabulary types", but a union that types nothing is dead code.

Recommendation: delete the unreferenced ones here. Where the narrowing is wanted back (turnStatus, sessionStatus, role, provider), the fix is a Rust enum in that type's own issue, which then generates the union.

Principle: CLAUDE.md "delete unused code"; ADR-086 §8 (hand-written TypeScript keeps only guards, converters, aliases and helpers).

🟢 Suggestions

4. Custom-Serialize enum literals have two sources
📁 packages/nodespace-types/src/task.rs:6-18,59-72, schema.rs:154-170

The TypeScript literals come from #[ts(rename_all = …)]; the wire strings come from as_str(). They agree today, but nothing pins them: changing as_str() regenerates identical TypeScript and the drift check passes. The generator could assert at run time that each SchemaFieldType::ALL[i].as_str() (and each named TaskStatus/TaskPriority variant) appears in T::decl(cfg). (DRY.)

5. NodeEnvelope.title is widened in Rust to describe a TypeScript producer
📁 packages/nodespace-types/src/node.rs:49-53, packages/dev-tools/src/dev-proxy.ts:333

Rust omits title when it is None. The | null exists only because the dev-proxy sends title: n.title === undefined ? null : n.title. Having the proxy omit the key instead would let the envelope drop optional = nullable and make the generated type exactly what serde emits. More broadly, the dev-proxy is a second producer of these shapes that no gate checks; that is worth keeping in mind, not something to solve in this PR.

6. Pre-existing, now more visible: two | null update fields that serde does not honour
📁 packages/nodespace-types/src/task.rs:147-148 (TaskNodeUpdate.priority), node.rs:233-234 (NodeUpdate.title)

Both are Option<Option<T>> with no clearable deserializer. As I read serde's default behaviour, JSON null becomes outer None ("unchanged"), not Some(None) ("clear"). The other tri-state fields use deserialize_clearable or flexible_date. The generated priority?: TaskPriority | null and title?: string | null therefore advertise a clear path that does not exist. I did not write a test to confirm this; please verify. If it holds, adding default, deserialize_with = "deserialize_clearable" is a small fold-in and does not change the wire shape.

7. Docs (nodespace-docs)

• development/node-type-sequence.md does not name bun run gen:types or types:check; components/creating-node-types.md:31 only says "the type-generation script". Naming the commands makes the step actionable.
• ADR-086 §2 says "In TypeScript, NodeEnvelope is the base interface every node interface extends". The generated typed nodes inline the envelope instead (structurally assignable, but not extends, and nodeType is string). One sentence in the ADR would keep it accurate.

8. Nits

• Nit: package.json:50: "build:skill":"bun run … lost the space after the colon (unrelated line touched).
• Nit: packages/desktop-app/src/tests/types/core-node-types.test.ts:1,11-13: leftover blank first line, a blank line inside the import list, and no blank line before describe. quality:fix runs eslint only, so Prettier never saw it.
• Nit: scripts/gen-types.ts:20: new URL(import.meta.url).pathname is percent-encoded for a path with spaces; import.meta.dir (as in build-skill.ts) avoids that.
• Nit: scripts/gen-types.ts:77-95: run() calls process.exit inside generate()'s try, so the finally that removes the temp directory does not run on a failed generation.
• Nit: scripts/gen-types.test.ts:144-148: asserting that the gate source contains the string run(TIERS.typesCheck) is brittle against a harmless refactor.
• Nit: gen_ts.rs:34-43: kebab splits every capital, so an acronym type name (URLThing) would become u-r-l-thing.ts. No current type is affected.

Frontend behaviour changes checked

• null to undefined for task dates: no === null / !== null reader of dueDate, startedAt or completedAt exists in src/lib. nodeToTaskNode still writes every typed key, so a spread-merge over an older copy still clears a removed date. The only dueDate: null left is in update patches, where null is the documented clear value.
• DeleteResult.deleted_count: no frontend code reads the field. The Tauri command (app-lib/src/commands/nodes.rs:764) returns the Rust struct, so the old deletedCount type was simply wrong.
• nodeType: string on typed nodes: the guards still narrow via isExactly. Nothing relied on literal discrimination, which CLAUDE.md bans anyway.
• Gate scope: a file under generated/ reaches both the Rust and frontend tiers (the rule sets rust and falls through); scripts/gen-types.ts reaches Rust and scripts; .prettierrc reaches Rust and frontend. This matches scripts/gate-scope.ts:109.

Security / Performance

• Security: no concerns. This is build tooling only; ts-rs is a dev-time, feature-gated dependency pinned in Cargo.lock, and there is no runtime surface.
• Performance: no runtime impact. Gate cost is one small crate build plus a Prettier pass, placed first in the Rust lane so drift fails fast.

Recommendation: APPROVE

Findings 1-3 are worth addressing in this PR but do not block the merge; the rest are optional.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Review addressed

Addressed

• 1. Coverage scan fails open, untested. The scan moved from the Rust binary to scripts/gen-types.ts (wireTypesInSource, missingWireTypes), where bun:test covers it with fixture strings. It now walks src/ recursively, counts Serialize or Deserialize (derived or hand-implemented), reads through comments above a declaration, and no longer cuts the file at the first #[cfg(test)]: only column-zero declarations count, which excludes test fixtures wherever the module sits. A scan that finds nothing fails. Checked by hand: a new serialized struct with no derive makes bun run types:check fail naming the type and its file.
• 2. Generator outside clippy. rust:lint now also runs clippy on nodespace-types with --features ts --all-targets.
• 3. Orphaned vocabulary types. Deleted AiChatTurnStatus, AiChatSessionStatus, LifecycleStatus, ProjectStatus and the ProtectionLevel alias (the index now exports SchemaProtectionLevel). AiChatProvider stays; three modules use it.
• 4. Enum literals with two sources. gen-ts now refuses to run unless every value serde writes for a named TaskStatus / TaskPriority / SchemaFieldType variant is a literal of the generated union.
• 6. null did not clear TaskNodeUpdate.priority or NodeUpdate.title. Confirmed and fixed with deserialize_clearable; each has a tri-state test (absent / null / value). The wire shape is unchanged.
• Nits: package.json spacing, the test file's blank lines, import.meta.dir, and the temp directory is now removed when generation fails (run throws instead of exiting inside the try).

Skipped

• 5. Make the dev-proxy omit title. That changes a browser-mode wire shape, which the issue lists as a non-goal. title?: string | null describes both transports as they are.
• Nit: string assertion on the gate sources. Kept: it is the only test that the merge gate and test:changed run the stage, and importing either script would run it.
• Nit: kebab and acronym type names. No such type exists; the script's generatedFileName mirrors the same rule, so a mismatch would fail the coverage check.

Outside this repo

• 7. Docs. Naming bun run gen:types / types:check in node-type-sequence.md and creating-node-types.md, and the ADR-086 §2 sentence about typed nodes inlining the envelope, go to nodespace-docs separately.

Verification: bun run quality:fix clean; bun run test:changed (every tier) passes: frontend, scripts, skill, browser, types:check, Rust workspace.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: The review approved with no critical findings. The follow-up changes are a relocation of the coverage scan into tested script code, a lint-script addition, deletions of unreferenced types, a generator assertion, and two one-attribute deserializer fixes each pinned by a test. None changes a design decision the review assessed, and the full test:changed run passes on the result.

[malibio]

Merge queue: #3471 was taken out of the queue — it conflicts with main or with #3470, queued ahead of it, in: packages/nodespace-types/src/person.rs, packages/nodespace-types/src/project.rs, packages/nodespace-types/src/task.rs. Rebase onto origin/main, push, and re-run bun run merge 3471.

[malibio]

Rebased onto main after #3470 landed (squashed to one commit). #3470 replaced TaskPriority with the shared Priority and made TaskNodeUpdate schema-fields-only, so: Priority now derives TS (generated priority.ts replaces task-priority.ts), the frontend's TaskPriority references are renamed, and the review's priority-clears-on-null fix is dropped as #3470 already made it. bun run quality:fix clean and bun run test:changed (every tier) passes on the rebased branch.