feat(tasks): add subtask_progress to every vault_list_tasks entry - #536
Conversation
Filtered or top_level_only board reads could not see whether a card has
a checklist or how far along it is. Every entry now carries
subtask_progress: { done, total } over its direct checklist children —
{ done: 0, total: 0 } means no checklist, done counts status done only,
and the counts ignore the query's filters.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
|
umm-actually re-reviewed at No new findings (1 tracked finding(s) across all runs). Context notes
umm-actually · deepseek/deepseek-v4-flash-0731 |
A partial index on tasks(note_path, parent_line) WHERE parent_line IS NOT NULL lets the grouped aggregate scan only child rows in GROUP BY order instead of hash-aggregating the whole tasks table on every listTasks call (EXPLAIN QUERY PLAN: temp B-tree eliminated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
@coderabbitai review 🔍 ship-check · pr-monitor · claude-fable-5 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesTask subtask progress
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The task progress contract is implemented and covered across query, mapping, and tool layers with no actionable merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
|
Present only when the task has direct checklist children — absence means
no checklist, exactly as unambiguous as { done: 0, total: 0 } while
saving the field on the majority of rows (most tasks are leaves), which
matters for LLM clients reading large filtered pages. Matches the wire
convention that metadata a task doesn't have is omitted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review Requesting an incremental review of 6da58dc — subtask_progress is now omitted for tasks without a checklist (previously always-present { done: 0, total: 0 }). 🔍 ship-check · pr-monitor · claude-fable-5 |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Commit metadata and changed files ==='
git show --no-ext-diff --format=fuller --stat 6da58dc4
echo
echo '=== Commit patch ==='
git diff --no-ext-diff --find-renames 6da58dc4^ 6da58dc4 -- \
ARCHITECTURE.md \
src/vault-mcp/search/search-index.ts \
src/vault-mcp/search/search-helpers.ts \
src/vault-mcp/search/search-queries.ts \
src/vault-mcp/search/__tests__/task-queries.test.ts \
src/vault-mcp/search/__tests__/search-helpers.test.ts \
src/vault-mcp/mcp-core/tools/task-tools.ts \
src/vault-mcp/mcp-core/__tests__/tool-definitions.test.ts \
src/__tests__/integration/server-integration.test.ts
echo
echo '=== Changed TypeScript file outlines ==='
git diff --name-only 6da58dc4^ 6da58dc4 -- '*.ts' |
while IFS= read -r file; do
[ -z "$file" ] && continue
echo "--- $file ---"
ast-grep outline "$file" --items all || true
done
echo
echo '=== All subtask_progress references with context ==='
rg -n -C 4 '\bsubtask_progress\b|\bsubtask_done\b|\bsubtask_total\b' \
src ARCHITECTURE.mdLength of output: 41881 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== vault_list_tasks registration and response construction ==='
ast-grep run --pattern 'registerTool($$$)' --lang typescript \
src/vault-mcp/mcp-core/tools/task-tools.ts || true
rg -n -A80 -B12 '\bvault_list_tasks\b|listTasks\s*\(' \
src/vault-mcp/mcp-core src/vault-mcp/search \
-g '*.ts'
echo
echo '=== TaskEntry construction and consumption ==='
rg -n -C 5 '\bTaskEntry\b|rowToTaskEntry\s*\(' src -g '*.ts'
echo
echo '=== Output schema and JSON serialization sites ==='
rg -n -C 5 'JSON\.stringify|structuredContent|outputSchema|z\.object|serialize' \
src/vault-mcp/mcp-core src -g '*.ts'
echo
echo '=== Remaining former-contract wording ==='
rg -n -i -C 2 \
'subtask_progress.*every|every.*subtask_progress|progress.*every entry|no checklist.*\{ done: 0, total: 0 \}' \
. -g '!node_modules/**' -g '!dist/**' -g '!build/**'Length of output: 200963
I found no actionable issues. The optional I did not run tests in the sandbox.
|
1 participant
What
vault_list_tasksentries gainsubtask_progress: { done, total }— the task's direct checklist children, aggregated in the same query.donecounts children with statusdoneonly; a cancelled child counts towardtotalbut notdone(three abandoned stages should not read as progress).top_level_onlyread still shows each card's full checklist progress, because progress is a property of the card, not of the query.Why
A filtered or limited board read (
folder+ a due bound +limit, ortop_level_only: true) could not see whether a card has a checklist or how far along it is: checklist items are separate depth-1 rows that either compete with real cards for result slots or disappear entirely, and the parent row said nothing about them.How
One grouped self-join in
listTasks(search-queries.ts): a LEFT JOIN on(note_path, parent_line)computingCOUNT(*)andSUM(status = 'done')per parent. The group key is unique per parent, so the join never multiplies rows, and the aggregate subquery sits outside the outer WHERE by design. A partial index ontasks(note_path, parent_line) WHERE parent_line IS NOT NULLkeeps the aggregate an index-only scan over child rows (EXPLAIN QUERY PLANverified — no full-table hash aggregation).TaskRow/TaskEntrygain the fields,rowToTaskEntrymaps the pair conditionally, and the tool description's Returns line documents the contract. ARCHITECTURE.md's task-query section gains the design bullet.Tests
task-queries.test.ts): leaf → field omitted; mixed checklist (2 done, 1 todo, 1 cancelled) →{2,4}under the defaultnot_donefilter, which also fails if the aggregate ever inherits the query's filters (the done/cancelled children are excluded from the result rows there); grandchild counts toward its direct parent only;top_level_onlyrows carry the pair; directrowToTaskEntrymapping test for non-zero counts.server-integration.test.ts): fixture board card with a mixed-status checklist asserted over real HTTP — exact pair on the parent, and a"subtask_progress" in taskguard on the parsed JSON proving the key is genuinely absent from leaf entries on the wire.SUM(status = 'done')→'cancelled'fails the cancelled-vs-done test; dropping theCOALESCEfails the leaf test; forcing the field to always emit fails 8 tests.Live validation
Exercised against a live test deployment of this branch (
test_deploy.yml→ the real instance) through a production MCP connector:{ done: 0, total: 1 }; cards without checklists carried nosubtask_progresskey.{ done: 0, total: 3 }; completed one stage viavault_update_task→{ done: 1, total: 3 }under the defaultnot_donefilter, with the completed child excluded from the result rows but still counted — filter-independence confirmed live.{1,2}; removing the done stage →{0,1}; removing the last stage → field absent. Counts always reflect the current file with no staleness (server-side writes index synchronously).🤖 Generated with Claude Code