[malibio]

Summary

Decision: align. project.priority now uses the same highest/high/medium/low/lowest scale as task.priority, and it sorts by rank.

Sort bug folded in: before this change, project priority sorted as plain text (high, low, medium). The query service ranked priority only when the target was task.

Changes

• Priority enum (packages/core/src/models/priority.rs): core's TaskPriority moved here and renamed, since it now backs both types. Priority::NODE_TYPES = ["task", "project"] names the types that share the scale. The wire-level nodespace_types::TaskPriority and project's plain-string wire field are unchanged.
• Sort: the SQL ORDER BY CASE and the in-Rust re-sort both rank via Priority::applies_to(target_type). They must stay in lockstep. Wildcard queries and user-defined types' bare priority fields still sort as text, as before.
• Schema: project.priority's core_values and description now match task's.
• Drift guards:
  • test_priority_variants_match_core_values_bidirectionally runs for every NODE_TYPES schema and pins the rank order.
  • New test_every_core_priority_field_uses_the_shared_scale fails if a core schema declares priority without being listed in NODE_TYPES.
• Test fix: the existing task LIMIT sort test used LIMIT 2. The alphabetical top 2 (high, highest) are the same rows as the rank top 2, so the test passed even against a text-sorting SQL layer. Both the task and project LIMIT tests now use LIMIT 3.
• The frontend project schema fixture is updated in lockstep.

Verification

• Mutation-checked:
  • SQL layer reverted to task-only → both LIMIT tests fail.
  • Rust comparator reverted to task-only → both project sort tests fail.
  • project dropped from NODE_TYPES → the reverse guard and the project sort tests fail.
  • Stray project core value → the drift guard fails.
• bun run test:changed: frontend, browser, scripts and Rust tiers all pass.

Docs

nodespace-docs will be updated on main after this PR merges:

• node-types.md: the enum name.
• data-layer.md: the sort-scope paragraph.

There is no project schema section in node-types.md to update, contrary to the issue's acceptance criteria.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review — PR #3253 (closes #2694)

Review Type: Initial Review

Summary

A clean, well-scoped change. project.priority now uses the same 5-level scale as task.priority. There is one Priority enum in models/priority.rs, and Priority::NODE_TYPES / applies_to gate both sort paths (SQL resolve_order_field and Rust compare_priority_values), so the two layers can't drift apart per type. The drift guards are stronger than before: an order-sensitive exact match for each type, plus a reverse check that fails when a core schema declares priority without being listed. The LIMIT 2 → LIMIT 3 fix closes a real vacuous-test gap. Wildcard and user-defined types still sort as text, and a venue assertion now pins that. No backward-compat code, no lint suppression, and no issue numbers in the new comments.

Requirements Check

• ✅ Decision made and recorded. Aligned. The PR and commit record it, and the Priority module doc and the new reverse-guard test state it in code. The written record in nodespace-docs is still to come (see the last criterion).
• ✅ project.priority core_values and prose description updated (core_schemas.rs). The description now also covers rank sorting and where user values sort.
• ✅ Project schema fixture updated in lockstep. generic-schema-form-project.test.ts matches the new core_values order exactly.
• ⏳ nodespace-docs/components/node-types.md project schema reference. No project schema section exists, so there is nothing to update for project itself. Line 182 still says TaskPriority and needs renaming. The deferral is acceptable (docs are committed straight to main), but it must happen right after merge, because of the next point.
• ✅ Drift guard considered. Added and stronger than the task-only original: exact equality in rank order for each type in NODE_TYPES, plus test_every_core_priority_field_uses_the_shared_scale.

Docs note: architecture/data-layer.md:279 currently says the rank applies only to task and that "project.priority is a separate scale". That becomes factually wrong the moment this merges. It is a two-sentence edit; do it immediately after the merge lands.

Code Review Findings

1. The resolve_order_field doc comment's index claims are wrong for project, and idx_project_priority is missing
📁 packages/core/src/services/query_service/mod.rs:647-669, packages/core/src/db/schema.rs:64-72
Severity: 🟡 Improvement

The comment explains that resolve_field is left untouched so that "equality filters on priority still hit the index" (idx_task_priority). It now covers both NODE_TYPES, but only task has a priority index. schema.rs has idx_project_status and no idx_project_priority, even though project.priority is declared indexed: true in core_schemas.rs:319. This gap predates the PR, but the PR is what extends the rationale to project, so the claim is now half-true.

Recommendation: add CREATE INDEX IF NOT EXISTS idx_project_priority ON node (json_extract(properties, '$.project.priority')) WHERE node_type = 'project'; next to idx_project_status. It is a one-line DDL edit, allowed under CLAUDE.md's schema-bootstrap exception; reset the DB. Then reword the comment to "the idx_<type>_priority indexes" (or name both). The cheaper alternative is to reword the comment so it says project has no index, but the schema field already claims indexed: true, so adding the index is the consistent fix. (Principle: comments must stay true as scope widens. Declared metadata should match the physical schema.)

2. Stale function reference in the moved Priority::rank doc
📁 packages/core/src/models/priority.rs:91-94
Severity: 🟢 Nit

"see QueryService::resolve_order_field and compare_json_values, which both do exactly that": the rank-plus-value tie-break is in compare_priority_values (its key() returns (rank, s.clone())). compare_json_values is the plain-text fallback. This was carried over verbatim from task_node.rs, but since the doc is being rewritten anyway, point it at compare_priority_values.

3. Over-long doc line
📁 packages/core/src/services/query_service/mod.rs:651
Severity: 🟢 Nit

/// [Priority::NODE_TYPES] types is a string enum whose alphabetical order (high, highest, low, lowest,` is about 110 columns. rustfmt won't rewrap comments, so reflow it by hand to match the surrounding block.

4. The daemon helper name parse_task_priority now returns the shared Priority
📁 packages/daemon/src/services/node_service.rs:3105
Severity: 🟢 Nit (optional)

It is still only called on the task path, so the name is accurate for its caller. No change needed unless a project typed-update path starts using it. Mentioned only so the rename scope is a conscious decision, like the kept wire TaskPriority and project.priority: String. Both of those are sound choices: the wire project field stays schema-owned, as nodespace-types/src/project.rs documents.

Things verified (no action)

• The SQL and Rust sort gates are identical (field == "priority" && Priority::applies_to(target_type)), so LIMIT and non-LIMIT orderings agree for project. The SQL half is independently pinned by test_project_sort_by_priority_with_limit_ranks_in_sql at LIMIT 3.
• Wildcard * and user-defined types still sort as text. applies_to("*") is false, and the venue assertion replaces the old project one.
• Nothing else assumes a 3-level project scale. I grepped for it:
  • methodology/linear.rs extends task.priority only (its BASE is already 5-level).
  • SKILL.md and references/*.md mention only task's scale.
  • The agent golden prompt snapshots have no project priority vocabulary.
  • The frontend ProjectNode.priority is a plain string, and the form is schema-driven.
• The drift guards are meaningful. The equality check covers both directions and the order in one assertion. The reverse guard catches the exact drift Decide whether project.priority should match task.priority's 5-level scale #2694 described.
• Security and performance: no new input surface. The CASE rank path is unchanged for task and now applies to project, with the same transient-sort trade-off already documented.

Recommendation

APPROVE. This is a clear net improvement. Finding 1 is worth folding in before merge: it is a one-line DDL plus a comment reword and makes the documented index rationale true for both types. The rest are optional nits. Update data-layer.md:279 and node-types.md:182 in nodespace-docs right after merge.

[malibio]

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

[malibio]

Review addressed (89fb647)

• ✅ 🟡 Missing idx_project_priority: added the partial expression index in db/schema.rs and pinned it in schema_test's expected index list. The resolve_order_field doc now names both types' priority indexes.
• ✅ 🟢 Stale compare_json_values reference in Priority::rank's doc now points to compare_priority_values.
• ✅ 🟢 Over-long doc line reflowed.
• ⏭️ 🟢 parse_task_priority name: kept. It only serves the task path, so the name is still accurate.

bun run test:changed: all tiers pass. quality:fix: clean.

Re-review: not needed. One DDL line, pinned by the existing schema test, plus doc-comment edits. No logic changes.

nodespace-docs (node-types.md:182, data-layer.md:279) will be updated on main right after merge.