Add title_template support for schema-driven title computation

[malibio]

Closes #824

[malibio]

Code Review Summary

Review Type: Initial Review

This PR implements schema-driven title computation via a title_template field on SchemaNode. The feature is well-scoped, the commit is atomic, and the implementation removes all three TODO #824 markers it promised to resolve. The architecture is sound: a pure interpolation utility lives in utils/, schema persistence uses the existing titleTemplate property key in the flat JSON store, and compute_title() is a single private method that centralizes all title logic. This is a net improvement to the codebase.

A few issues require attention before merge, one of which is a meaningful correctness gap.

---

Requirements Check

Acceptance Criterion	Status
Add title_template field to SchemaNode model	✅
Implement template interpolation function ({field} replacement)	✅
Update title computation in NodeService to check schema for template	✅
Recompute title when properties change (not just content)	✅
Add validation for title_template syntax in schema creation	❌ — not implemented
Update core schema definitions if any should use templates	✅ (None set on all 12 core schemas)
Add unit tests for template interpolation	✅
Add integration tests for schema-based title computation	✅

---

Findings

[Improvement] Missing Acceptance Criterion: Template Syntax Validation

packages/core/src/services/node_service.rs — schema creation/update path

The issue explicitly calls for validation of title_template syntax. No validation exists: a template like {unclosed or {{nested}} is accepted silently. While the interpolation function handles malformed input gracefully (it passes through an unclosed { as a literal), the absence of validation means malformed templates stored in the DB will silently produce incorrect titles that are hard to diagnose.

A minimal check — verifying that every { has a matching } and the field name is non-empty — would satisfy the criterion and flag author mistakes at schema-save time rather than at title-read time. This should be added to the schema validation path.

---

[Improvement] Silent Error Swallowing in compute_title

packages/core/src/services/node_service.rs, lines 2132–2145

if let Ok(Some(schema)) = self.get_schema_node(&node.node_type).await {

The Err(_) arm is discarded with no logging. If get_schema_node fails due to a database error, compute_title silently falls through to the content-based fallback, producing a wrong title with no observable signal. In a service that otherwise propagates errors via ?, this inconsistency violates the principle of least surprise and will make debugging intermittent title corruption very difficult.

The appropriate fix is either to propagate the error with ? (consistent with the rest of the method) or, if fallback on DB failure is intentional, to log a warning before continuing.

---

[Improvement] Double Normalization on create_node_with_parent

packages/core/src/services/node_service.rs, lines 1568–1596

The comment acknowledges this: properties are normalized to namespace format to construct temp_node for compute_title, then the raw params.properties (unnormalized) are passed into the actual Node that gets written to DB — where create_node normalizes them again. This means the normalization result used for title computation and the normalization result used for storage are computed independently from the same input.

This is safe today because normalization is idempotent, but it creates two divergence points: if normalization logic ever changes, the title computed here could differ from what is actually stored. A cleaner approach would be to normalize once, use the result for both compute_title and the stored Node, and pass the normalized form to create_node (or have create_node skip redundant normalization when properties are already normalized). The comment "create_node will normalize again, but the result is idempotent" is accurate but essentially documents a latent inconsistency risk.

---

[Nit] Test Schema IDs Use Raw Strings Instead of UUIDs for the Schema Itself

packages/core/src/services/node_service.rs, tests title_template_tests module

create_custom_schema uses type_id (e.g. "customer", "customer2", "customer3") as both the schema ID and the node type. The tests work, but using sequential suffix variations ("customer2", "customer3") to avoid collisions between isolated tests suggests these tests may share a NodeService instance. If the test service is truly isolated per test (each create_test_service() call creates a fresh DB), the suffixes are unnecessary and create misleading names. If they do share state, the tests have a hidden ordering dependency. It is worth confirming which is the case and either removing the suffixes or making the isolation explicit.

---

[Nit] Interpolation Parser Allocates a Vec<char> for Every Call

packages/core/src/utils/markdown.rs, lines 124–148

let chars: Vec<char> = template.chars().collect();

For a hot path called on every node create/update with a title_template, allocating a heap Vec<char> on each invocation is unnecessary. The same scan can be done with byte-index arithmetic on the original &str since { and } are single-byte ASCII. This is a minor allocation concern, not a correctness issue, and is low priority for a pre-release codebase, but worth a note for when performance tuning begins.

---

Final Recommendation

REQUEST CHANGES

The missing validation for title_template syntax is an explicit acceptance criterion that was not implemented. The silent error swallowing in compute_title is a meaningful correctness and observability gap. Both should be addressed before merge. The double normalization is a design cleanliness issue worth tracking but not a blocker.

🤖 Generated with Claude Code

[malibio]

See detailed review comment above. Two issues to address before merge: (1) missing title_template syntax validation (explicit acceptance criterion), (2) silent error swallowing in compute_title when get_schema_node fails.

[malibio]

Review Feedback Addressed

✅ Addressed (2/4 findings)

🟡 Missing title_template syntax validation — Added to SchemaNodeBehavior::validate_schema_node() in behaviors/mod.rs. Rejects unclosed { placeholders and empty {} placeholders at schema creation/update time. Added 4 unit tests covering valid template, unclosed brace, empty placeholder, and None cases.

🟡 Silent error swallowing in compute_title — Replaced silent Err(_) discard with tracing::warn!. Schema lookup failures are now logged and observable without blocking the create/update operation (fallback to content-based title is intentional).

⏭️ Skipped (2/4 findings)

🟢 Double normalization in create_node_with_parent — Safe and idempotent today. Fixing it would require threading normalized properties through the rest of create_node_with_parent (which currently uses params.properties to build the stored Node), adding complexity for negligible benefit. Will revisit if normalization logic diverges.

🟢 Vec<char> allocation in interpolate_title_template — Not a hot path (title computation happens only on node create/update). Premature optimization without measurement.