Split SchemaField into name/friendly_name/description

[mstomar125]

Summary

SchemaField had name (storage/query key) and description doing double duty as both the UI display label and the LLM-facing semantic explanation — two purposes in direct conflict (a good label is a bad LLM description). This adds a third field, friendly_name, as the dedicated display label, freeing description to become genuine LLM-facing prose.

• friendly_name: String — non-optional in storage, always populated. Every UI reader uses it unconditionally; no fallback to description, no null-branching.
• Defaulting happens once, at the write boundary (create_schema/update_schema): if a caller omits friendly_name, it's derived from name (due_date → Due date, custom:capacity → Capacity, namespace prefix stripped for display), and a derived value that would collide with another field's label in the same schema is disambiguated (see below).
• description is now documented (Rust doc comment + TS JSDoc) as LLM-facing prose: meaning, purpose, usage, an example — consumed by schema retrieval/comprehension, never rendered as a label.

Closes #2101.

Design decisions (documented for the reviewer, not just the diff)

1. The seeder question — requires a database reset, not seeder reconciliation

The issue asked me to explicitly resolve this and document the reasoning, and pointed at precedent: 06a94eee ("Seeded content never reaches an existing install…") built a per-node versioned _seed reconciliation mechanism for exactly this class of gap, for prompt/skill/tool nodes.

I did not extend that pattern (or seed_core_schemas_if_needed) to reconcile friendly_name onto existing core schemas. Reasoning:

• The 06a94eee mechanism reconciles system-owned seeded content (prompt/skill/tool bodies) — content NodeSpace ships and can safely replace wholesale on a hash mismatch. friendly_name is a type-level storage format change: SchemaField previously had no such field, and it's non-optional going forward. That's not a "this specific seeded node's content changed" problem, it's "every schema field ever persisted, core or user-created, is missing a piece of shape the type now expects."
• A seed_core_schemas_if_needed reconciliation pass, even if built, would only touch the ~18 core schemas. It would do nothing for any user-created custom schema already in a dev's local database — which has the exact same gap (fields persisted before this change lack friendlyName). Shipping a fix that visibly repairs core schemas while silently leaving custom schemas broken is worse for dev experience than an honest "reset your database," because it looks complete and isn't. (An independent review pass traced SqliteStore::get_schema_node/get_all_schemas and confirmed both core and custom schemas go through the identical code path with no is_core distinction — this claim isn't asserted on faith.)
• Building a general reconciliation pass covering arbitrary pre-existing schema data (not just the 18 known core ones) is real migration/back-compat code — exactly what this repo's CLAUDE.md forbids pre-release ("NO backward compatibility code," "NO migration strategies — we can reset the database anytime"), and what the issue's own design section already anticipated ("No backward compatibility. Pre-release, no users, no migration — reset the database.").

What actually happens on a non-reset database: friendly_name has #[serde(default)] at the type level (needed anyway so create_schema/update_schema callers can omit it), so a legacy field without friendlyName in its stored JSON deserializes successfully with friendly_name: "" rather than failing the parse — no data loss, no crash. The visible symptom is blank column headers/labels on schemas that predate this change, which is a much softer failure mode than I initially expected before checking (I first assumed the whole fields Vec would silently deserialize to empty via an existing .ok()-swallowing bug in SchemaNode::from_node — see below — but that only triggers on an outright parse error, and the #[serde(default)] prevents that specific one). Still: reset your local database after pulling this change to get real labels back on every schema.

While tracing that swallow path, I found two structurally identical pre-existing sites that silently drop to an empty Vec<SchemaField> on any deserialize failure with no logging: packages/core/src/models/schema_node.rs::SchemaNode::from_node and a distinct SchemaNode::from_node in packages/nodespace-types/src/schema.rs (used by convert.rs::node_to_typed_value, the canonical node→wire-JSON path for Tauri/MCP/HTTP). I fixed the packages/core one (a tracing::warn! on parse failure). I deliberately did not fix the nodespace-types one in this PR — that crate has no logging dependency by design, and the right fix (propagate the error vs. add a diagnostic hook vs. a downstream healthcheck) has real design tradeoffs against nodes_to_typed_values's batch-collect() error semantics that deserve their own review, not a rushed addition here. Filed as #2107.

2. The Default/builder trap — #[derive(Default)] + struct-update syntax, not a builder

The issue flagged 69 bare SchemaField struct literals with no Default and no builder, making every future field addition another 69-site change. An independent count (excluding function-signature lines that merely return SchemaField and end in -> SchemaField {, which a naive grep for SchemaField { also matches) found 64 genuine construction sites: 48 in core_schemas.rs, plus markdown.rs (4), schema/mod.rs (1 helper), context_ops.rs (3), entity_types_block.rs (1 helper), schema_node.rs (1), and nodespace-types/src/schema.rs (6 test literals). All 64 updated.

I added #[derive(Default)] to SchemaField (and to SchemaProtectionLevel, via #[default] on the User variant, since it's a required non-Option field of SchemaField) rather than a hand-written builder. Reasoning: every optional field on SchemaField (13 of them) already has a sensible zero value (None/false), so #[derive(Default)] + Rust's ..Default::default() struct-update syntax gives every future field addition the same ergonomic win a builder would, with zero new methods to write or maintain. A builder would need ~13 setter methods to cover the same surface for no additional safety (there's no invariant here that a builder's fluent API would enforce that a plain struct literal can't).

This PR necessarily touches all 64 sites anyway (friendly_name itself has no useful default — every site had to decide a real value), so I did not retrofit every existing literal to use ..Default::default() for its already-None fields — that would have made this diff considerably larger for a purely cosmetic win on lines untouched by this change. What matters for the stated goal is that the capability now exists: the next field addition can be written as SchemaField { name: ..., friendly_name: ..., new_field: ..., ..Default::default() } at each site instead of enumerating all 17 fields, which is the actual "not a 69-site change" property being asked for — a new field only needs ..Default::default(), not a new value at every site (unless that site wants a non-default value).

3. 64-call-site migration — confirmed complete, independently re-verified

cargo check --workspace --tests compiles clean, cargo clippy --workspace --all-targets is clean. A second, independent review pass re-derived the count from scratch (its own repo-wide grep + a brace-matching script, not trusting this description) and confirmed all 64 sites set friendly_name explicitly, none rely on a silently-empty default. It also re-checked every TS/Svelte SchemaField object-literal site the same way, including two as SchemaField type-assertion casts in test files that had been silently hiding a missing-required-field gap (friendlyName, and in one case indexed too) — both replaced with a real default so the compiler actually checks the shape now.

4. Core schema description rewrites

Rewrote task's 6 fields and project's 4 (the exact same terse-label pattern as task, not explicitly named in the issue but an obvious companion — e.g. project.status was literally Some("Status")) into real LLM-facing prose. person's descriptions were already genuine prose (that's the issue's own positive example) — only needed friendly_name added (Name/Email). Left the remaining schemas (ai-chat, query, skill, database-settings, collection) with their existing descriptions, which were already explanatory prose rather than label-like one-word text, aside from adding friendly_name (mechanically derived from the existing short label text where one existed, or humanized from name otherwise) — per the issue's own guidance to prioritize the mechanism over exhaustively rewriting every schema.

5. Deliberately out of scope: packages/agent

A comment on #2101 expanded scope to the agent-facing create_schema/update_schema tool JSON schemas (packages/agent/src/local_agent/tools.rs) and the schema-authoring skill guidance (packages/agent/src/skill_pipeline.rs) — the LLM still needs to be told friendly_name exists and that description now wants real prose. I did not touch packages/agent — it's a different engineer's lane by this repo's ownership convention, and this PR stays inside packages/core, packages/nodespace-types, and packages/desktop-app. Filed as a follow-up: #2104.

The core plumbing this depends on is already in place and tested: SchemaField accepts friendlyName from any caller's JSON today (optional, defaulted server-side when omitted), so #2104 is purely "teach the agent this parameter and the new description semantics exist" — no further packages/core changes should be needed.

Adversarial review — two passes, real defects found and fixed

Given the scope, I ran two independent review passes (Rust correctness/design vs. frontend/migration-completeness) before merging.

Frontend/migration reviewer: APPROVE, minor findings only (the 68→64 count above, the TS unique/uniqueCaseInsensitive gap, a cosmetic double-blank-line) — all addressed in this PR.

Rust reviewer: REQUEST CHANGES, and correctly so — it found two real bugs, not just polish:

1. Namespace-prefix stripping collided with an existing field today, contradicting my original doc comment's claim that this was only a future hypothetical. update_schema adding custom:status to any schema that already has a bare status field derived "Status" for both — same displayed label, different storage keys, no way to tell them apart in any UI surface. Fixed: apply_friendly_name_defaults now takes the full existing-plus-in-batch field set and disambiguates a derived collision by appending the namespace ("Status (custom)") or the raw field name when there is none. An explicitly-supplied friendly_name is never touched even if it collides — that's the caller's deliberate choice, not this function's to override.
2. derive_friendly_name's camelCase splitting mishandled an acronym adjacent to another word — employeeIDNumber merged into "Idnumber" instead of splitting into three words, because the boundary detector only handled a single lower→upper transition, not the acronym-run case. Fixed with the standard second boundary rule.

Both fixes have dedicated regression tests (5 new: 2 unit tests for the acronym case, 3 integration tests for collision disambiguation — existing-field collision, in-batch self-collision, and confirming an explicit colliding value is never rewritten).

The reviewer's other findings: the nodespace-types unfixed swallow site (discussed above, filed as #2107) and rename_schema_field leaving friendly_name unrefreshed after a rename (documented as a deliberate tradeoff via a doc comment — re-deriving it unconditionally on rename risks clobbering an explicit caller choice with no way to distinguish the two cases).

What changed

Rust

• packages/nodespace-types/src/schema.rs — SchemaField.friendly_name: String (#[serde(default)]), derive_friendly_name() (with acronym-boundary handling), #[derive(Default)] on SchemaField/SchemaProtectionLevel, doc comments distinguishing name/friendly_name/description.
• packages/core/src/schema/mod.rs — apply_friendly_name_defaults() (defaulting + collision disambiguation), called from handle_create_schema and handle_update_schema's add_fields path with the appropriate existing-field context for each.
• packages/core/src/models/core_schemas.rs — all 48 field literals gain friendly_name; task + project descriptions rewritten as LLM-facing prose.
• packages/core/src/models/schema_node.rs — diagnostic log on the pre-existing silent field-parse-failure swallow.
• packages/core/src/services/node_service/schema.rs — doc comment on rename_schema_field's friendly_name-staleness tradeoff.
• Remaining 15 construction sites across markdown.rs, schema/mod.rs, context_ops.rs, entity_types_block.rs, nodespace-types/src/schema.rs test module.

TypeScript/Svelte

• packages/desktop-app/src/lib/types/schema-node.ts — friendlyName: string (required), unique/uniqueCaseInsensitive (were missing from the mirror), updated JSDoc.
• packages/desktop-app/src/lib/utils/schema-field-label.ts — new shared labelForField() helper (single source of truth for "how do I get a field's UI label").
• table-view.svelte, kanban-view.svelte, query-editor-model.ts/query-editor.svelte, schema-property-form.svelte, task-schema-form.svelte, generic-schema-form.svelte, nested-field-editor.svelte, nested-property-modal.svelte, schema-field-leaf.svelte — all now call labelForField() instead of field.description || <local regex>. Five independently-duplicated name-humanizing regexes deleted. Value-humanizing helpers that format an actual value (not a field name) — schema-property-form's/schema-field-leaf's enum-value fallback formatters — were left in place; that's a different, still-legitimate concern, independently re-verified line-by-line by the review pass.
• table-view.svelte's stale doc comment (claimed "field.label", no such field existed) corrected.
• Synthetic SchemaField descriptors in nested-field-editor.svelte (array-item recursion) now carry friendlyName.

Tests

• New: derive_friendly_name unit tests (10 cases, including the acronym-adjacent-word fix and an all-uppercase case), SchemaField serde round-trip tests, 7 handle_create_schema/handle_update_schema integration tests covering write-boundary defaulting (omitted → derived, explicit → respected) and collision disambiguation (existing-field collision, in-batch self-collision, explicit-value-never-rewritten).
• New: labelForField() unit tests, a table-view.svelte header-rendering test using the person schema as the concrete "friendly_name not description" regression case (Name/Email headers, verbose description text asserted absent).
• Updated: every existing test fixture constructing a bare SchemaField (64 Rust sites + ~15 TS test files) to supply friendly_name; two TS test helpers previously used an as SchemaField cast that silently hid a missing-required-field gap — replaced with a real default.

Testing

• cargo test -p nodespace-types -p nodespace-core --lib: all pass (1152 in nodespace-core, 45 in nodespace-types, includes all new tests above)
• cargo test -p nodespace-agent -p nodespace-daemon -p nodespace-cli --lib: all pass (470 / 13 / 147), confirming no regression in crates that consume core_schemas.rs output but weren't touched
• cargo clippy --workspace --all-targets: clean
• cargo fmt --check (touched crates): clean
• bunx vitest run (full suite, cold cache): 213 files / 4666 tests pass
• bunx svelte-check: 0 errors across 2272 files
• bunx eslint: clean

Acceptance criteria

• SchemaField carries name, friendly_name, description with distinct documented purposes
• friendly_name always populated in storage; derived from name at the write boundary when omitted
• No read-time fallback or null-branching on friendly_name in any UI surface
• Person query headers read Name / Email; task headers unchanged
• Core schema descriptions rewritten as genuine LLM-facing descriptions (task, project, person)
• All UI label sites read friendly_name via one shared helper; duplicated regex removed
• SchemaField gains Default so future field additions are not 69-site changes
• Seeder question resolved explicitly and documented (this section)
• deny_unknown_fields still satisfied; Rust and TS types in sync
• Schema-editing UI allows authoring friendly_name — via the packages/core write boundary today (any caller, human or agent, can already pass friendlyName in create_schema/update_schema JSON); the agent's own tool-schema exposure of that parameter is Expose friendly_name in create_schema/update_schema tool schemas + skill guidance (agent lane, follow-up to #2101) #2104, deliberately deferred per the lane boundary above

Co-Authored-By: Claude noreply@anthropic.com

[mstomar125]

APPROVE

Independently re-verified after both review passes and the fix commit (569564c):

Diff review: read the full diff against main (35 files, +895/-108) file by file. Scope is exactly what the issue and PR description claim — no accidental touches to packages/agent, nlp-engine, skill, or any other excluded lane (confirmed via git diff --stat and a targeted grep for friendly_name/SchemaField under packages/agent/src, which returns nothing).

64-call-site migration — confirmed complete, independently, a third time: cargo check --workspace --tests and cargo clippy --workspace --all-targets both clean. Re-ran the frontend reviewer's independent count method myself (repo-wide grep for SchemaField { excluding function-signature false positives) and got the same 64. Every site sets friendly_name explicitly; none rely on a silently-empty default.

Seeder decision — sound: requires a database reset, not seed_core_schemas_if_needed reconciliation. Verified the core claim myself: SqliteStore::get_schema_node/get_all_schemas use the identical code path for core and user-created schemas (no is_core branch in the fetch), so a reconciliation pass scoped to the 18 known core schemas would leave every custom schema in the same stale state while looking complete — worse for dev experience than an honest reset. friendly_name's #[serde(default)] means the actual failure mode on a non-reset DB is blank labels, not data loss (verified by reading SchemaField's serde attributes and the .ok()-swallow site in schema_node.rs, which now also logs on genuine parse failure).

Both real defects from the Rust review pass are fixed and independently re-verified as fixed:

• Namespace-collision disambiguation: read apply_friendly_name_defaults/disambiguate_friendly_name line by line, confirmed the collision-context threading is correct for both handle_create_schema (empty existing-set) and handle_update_schema (existing schema fields post-removal, pre-extend) — this was the trickiest part of the fix (had to move the defaulting call to a point in handle_update_schema where the full field context exists, per the review's finding) and I traced the control flow myself rather than trusting the diff. Ran the new test_update_schema_add_fields_disambiguates_friendly_name_colliding_with_existing_field, test_create_schema_disambiguates_friendly_name_colliding_within_same_batch, and test_update_schema_add_fields_does_not_rewrite_an_explicit_colliding_friendly_name tests directly — all pass.
• Acronym-boundary bug in derive_friendly_name: traced the two-rule boundary detector by hand against employeeIDNumber and confirmed it produces ["employee", "ID", "Number"] before lowercasing, matching the new test assertions. Ran test_derive_friendly_name_splits_acronym_adjacent_to_next_word and test_derive_friendly_name_all_uppercase_is_treated_as_one_word directly — both pass.

Full test re-run (not just trusting CI/prior runs):

• cargo test -p nodespace-types -p nodespace-core -p nodespace-agent -p nodespace-daemon -p nodespace-cli --lib: one transient failure on the first pass (test_bulk_create, unrelated to schemas — creates plain text/task nodes, no SchemaField involved), reproduced clean in isolation and on a full re-run (1152/1152), consistent with resource contention from running 5 crates' test binaries concurrently rather than a real regression.
• cargo clippy --workspace --all-targets: clean.
• cargo fmt --check: clean for every file this PR touches; the 2 remaining diffs are pre-existing drift in desktop-app/src-tauri/src/commands/nodes.rs, a file this PR never touches.
• bunx eslint --fix + bunx svelte-check (quality:fix equivalent): zero errors, zero changes produced — nothing left to fix.
• bunx vitest run (full suite, not filtered): 213/213 files, 4666/4666 tests.

Merging.