refactor: task breadcrumbs follow-ups (shared codec, module extraction, fixtures)#713
Merged
Merged
Conversation
- One codec (encode/decodeTaskBreadcrumbs) for the tasks.breadcrumbs column, used by the dev-index writer and the query boundary - toTaskRow destructures raw fields instead of spread-overwriting - The ancestor-list walk moves out of extract.ts into markdown/task-breadcrumbs.ts - One shared makeOpenTask test fixture replaces eight local copies - groupTaskContexts precomputes visibleBreadcrumbs; the TaskBreadcrumbs component is purely presentational and visibleTaskBreadcrumbs leaves the public API - Plan 18 doc gains the canonical breadcrumb-semantics bullet Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
maccman
changed the title
WalkthroughTask breadcrumbs are now derived from list ancestry, encoded and decoded through shared indexing helpers, filtered into visible context labels, and rendered directly by desktop task groups. Multiple task tests now use a shared ChangesTask breadcrumb flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MarkdownParser
participant TaskBreadcrumbs
participant IndexWriter
participant TasksTable
participant TaskQueries
participant TaskContext
participant DesktopTaskGroup
MarkdownParser->>TaskBreadcrumbs: derive ancestor labels
TaskBreadcrumbs-->>IndexWriter: provide breadcrumbs
IndexWriter->>TasksTable: store encoded breadcrumbs
TasksTable-->>TaskQueries: return task row
TaskQueries->>TaskContext: decode breadcrumbs and group tasks
TaskContext-->>DesktopTaskGroup: provide visibleBreadcrumbs
DesktopTaskGroup->>TaskBreadcrumbs: render visible labels
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/lib/tasks/open-task-fixture.ts`:
- Around line 9-15: Update the fixture factory’s default raw-line construction
to derive the marker from the resolved checked value, so makeOpenTask({ checked:
true }) produces a checked marker while preserving any explicit raw override;
use the checked variable and raw field in makeOpenTask.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 117c9fe2-4a02-418c-acff-1b32de032197
📒 Files selected for processing (21)
apps/desktop/src/components/tasks/task-breadcrumbs.tsxapps/desktop/src/components/tasks/task-group-section.tsxapps/desktop/src/components/tasks/tasks-screen.test.tsxapps/desktop/src/dev/dev-index-db.tsapps/desktop/src/lib/tasks/open-task-fixture.tsapps/desktop/src/lib/tasks/recently-completed.test.tsapps/desktop/src/lib/tasks/task-cache.test.tsapps/desktop/src/lib/tasks/task-navigation.test.tsapps/desktop/src/lib/tasks/task-visibility.test.tsapps/desktop/src/lib/tasks/use-task-keyboard.test.tsapps/desktop/src/mobile/screens/tasks.test.tsxapps/desktop/src/mobile/use-task-sheet-finalizer.test.tsdocs/plans/18-tasks.mdpackages/core/src/exports/sync-markdown-indexing.tspackages/core/src/indexing/group-tasks.test.tspackages/core/src/indexing/group-tasks.tspackages/core/src/indexing/index.tspackages/core/src/indexing/indexed-note.tspackages/core/src/indexing/queries-tasks.tspackages/core/src/markdown/extract.tspackages/core/src/markdown/task-breadcrumbs.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
#685 landed task context breadcrumbs with a few structural loose ends, identified while comparing it against the competing #686:
tasks.breadcrumbsJSON column format was hand-rolled in three places (Rust writer, dev-index writer, query boundary) with nothing marking them as one contract.toTaskRowspread the raw SQL row and overwrote fields, so the storedbreadcrumbs: stringand the domainreadonly string[]shared a name across the boundary, and both callers double-spread the result.extract.ts, which was closing in on the repo's 500-line lint ceiling.OpenTaskfixture helpers — Add task context breadcrumbs #685 itself had to touch all of them just to add one field.TaskBreadcrumbscomponent at render time, while the rule itself lives in core.Changes
encodeTaskBreadcrumbs/decodeTaskBreadcrumbslive next toindexedTaskSchemain core and are the only way the TS side touches the column (dev-index writer +toTaskRow). Rust'swrite.rsmirror stays pinned by its existing serialization test.toTaskRowmaps raw → domain explicitly. Raw fields are destructured away;getOpenTasks/getCompletedTasksare nowrows.map(toTaskRow).markdown/task-breadcrumbs.ts. The walk (taskBreadcrumbs+ label helpers) moves out ofextract.ts(−52 lines) into its own module with a header comment stating the semantics. No behavior change; the dead!== undefinedarm in the ancestor loop is dropped.makeOpenTaskshared fixture. One helper inlib/tasks/open-task-fixture.tsreplaces the eight local copies;task-navigationanduse-task-sheet-finalizerkeep one-line wrappers for their differing default text.TaskContext.visibleBreadcrumbs.groupTaskContextsprecomputes the display labels, theTaskBreadcrumbscomponent becomes purely presentational, andvisibleTaskBreadcrumbsleaves the public core API (it was only exported for the component).Net −40 lines. No behavior, schema, or migration changes — pure structure.
Tests
language-model.test.tsfailure is the known pre-existing local env drift, untouched by this diff).lib/tasks, tasks screens, mobile tasks, sheet finalizer, dev-index-db, selection) — 203 passed.groupTaskContextslabels contexts withvisibleBreadcrumbs(trimmed real label vs suppressed generic).pnpm checkclean; the two max-lines warnings (indexer.ts,graph-provider.tsx) are pre-existing files this PR doesn't touch.Risk / Rollout
Refactor-only: no schema, projection, or parsing behavior changes, so no reindex is triggered. The public core API shrinks by one export (
visibleTaskBreadcrumbs), which nothing outside core consumed.🤖 Generated with Claude Code
Note
Low Risk
Refactor-only with no migration or projection bump; the only API shrink is dropping the unused
visibleTaskBreadcrumbsexport from core.Overview
Structural follow-ups to task context breadcrumbs (#685): no schema, parsing, or user-visible behavior changes.
Core:
encodeTaskBreadcrumbs/decodeTaskBreadcrumbscentralize thetasks.breadcrumbsJSON column;toTaskRowdecodes at the query boundary so the stored string type never leaks. Ancestor-list walking moves fromextract.tsintomarkdown/task-breadcrumbs.ts.groupTaskContextsnow exposesvisibleBreadcrumbs(generic-parent suppression stays in core);visibleTaskBreadcrumbsis no longer a public export.Desktop:
TaskBreadcrumbsonly renders precomputed labels;TaskGroupSectionpassescontext.visibleBreadcrumbs. Dev index writes use the codec. Eight test files sharemakeOpenTaskfromopen-task-fixture.ts.Docs: Plan 18 documents the full breadcrumb contract.
Reviewed by Cursor Bugbot for commit 0ce7bac. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation