feat(dependencies): acknowledge and surface status readiness for advanced dependency types - #20
Open
renatomen wants to merge 33 commits into
Open
feat(dependencies): acknowledge and surface status readiness for advanced dependency types#20renatomen wants to merge 33 commits into
renatomen wants to merge 33 commits into
Conversation
Add an optional StatusCategory (planned | in-progress | completed) to StatusConfig, kept in sync with the load-bearing isCompleted flag rather than replacing it. A load-time normalization backfills category=completed for already-completed statuses and leaves everything else uncategorized (no guess). StatusManager gains isStarted/isFinished/getCategory so the dependency cache can evaluate start-anchored constraints.
Replace the binary completed toggle in the status config card with a category dropdown (Uncategorized / Planned / In progress / Completed) and show the chosen category on the card pill, mirroring how the completed badge rendered before. Selecting Completed keeps isCompleted in sync; migrated statuses render a neutral Uncategorized nudge. Adds category pill styles and the i18n strings.
- Reconcile the isCompleted <-> category invariant in both directions in normalizeStatusCategories, so a completed status can never read as finished-but-not-started once the lifecycle predicates gain consumers. - Fold the duplicated categoryOptions strings into the shared badges keys the pill already uses; drop the orphaned completed toggle strings. - Cover the migration through the real settings load path, and add the completed-with-stale-category reconciliation case. - Record the status settings change in the unreleased changelog.
… Completed) Drop the fourth 'Uncategorized' state per the product decision. Every status now carries one of three lifecycle categories, guaranteed at runtime: the load-time normalizer defaults any non-completed status to Not started, and the two settings write sites (add-status button, createDefaultStatus) seed a category so nothing uncategorized is reachable. Relabel the enum's user-facing strings to Not started / Started / Completed; isCompleted stays the derived Completed mirror.
…cyCache Replace the reltype-blind completion-only gating with RFC 9253 per-endpoint constraints: FS/SS gate a task's start, FF/SF gate its finish, and each edge releases when its predecessor reaches its own endpoint (completed for FINISH*, started for START*). The cache stores each edge's reltype and both the finished and started lifecycle per path, computing startBlocked/finishBlocked on demand; the relationship fingerprint keys on reltype so a reltype-only edit re-indexes, and the file signature keys on the per-endpoint blocked membership so a predecessor's lifecycle move re-fires the change event. Exposes isTaskStartBlocked/isTaskFinishBlocked plus reverse per-endpoint accessors.
Add startBlocked/finishBlocked and the reverse isBlockingStart/isBlockingFinish to TaskInfo, sourced from the cache's per-endpoint accessors at both assembly sites (TaskManager, bases/helpers); isBlocked stays as their OR for existing consumers. The cache-absent fallback approximates start/finish-blocked from the task's own edge reltypes (existence-based). This is the contract U5 (surfaces) and U8 (Bases) consume.
…accessors The calendar-filter test mocks dependencyCache with only the legacy accessors; TaskManager's assembly now also calls isTaskStartBlocked/isTaskFinishBlocked and the reverse dependent-path accessors (U4), so the partial mock threw and the task dropped out of the filtered set. Add the missing methods to the mock.
Wire the task-card secondary badge, the metadata pill, and the editor widget to the computed start/finish-blocked state instead of raw blocked-by existence. A task reads "Blocked · start" when it cannot begin and "Blocked · finish" when it can begin but cannot complete; start wins over finish so the "can I act now?" signal stays unambiguous. The badge and pill count only currently-constraining predecessors, so a released edge (started SS, completed FS) drops the blocked label and shows as a listed-but-satisfied dependency rather than "Blocked (1)". DependencyCache gains getStart/FinishBlockingPredecessorPaths (the forward constraining view); taskCardRelationships.resolveBlockedConstraint is the shared honest-signal resolver every surface reads.
…modal Add per-row relationship controls to the task edit modal's Blocked-by and Blocking lists (gated by a new enableAdvancedDependencyTypes setting, off by default): a reltype dropdown, a structured lag (number + unit composed to an ISO-8601 duration), and a plain-language summary that names both concrete tasks so the direction can't be read backwards. Blocking rows source their real reltype/gap from the blocked task's own blockedBy (the canonical location) instead of assuming the default, and editing a Blocking edge writes the change back there — TaskService gains a "modify" action alongside add/remove, and the edit-change diff now reports modified edges. An exotic stored lag stays read-only so an edit never silently rewrites it. The row layout stacks on narrow and mobile widths.
Surface a toggle for enableAdvancedDependencyTypes under a new Dependencies group in the Features tab, mirroring the existing feature-toggle pattern. Off by default; turning it on reveals the reltype/lag authoring controls in the task edit modal.
…t filter limit TaskNotes-rendered Bases views (Task List, Kanban, cards) already carry the computed start/finish-blocked state from the cache. Add coverage for those fields, and document why a native .base filter on blocked-by stays existence-only: a filter formula evaluates one note's frontmatter and cannot read a predecessor's status, so it cannot tell a released edge from a live one. Update the dependencies docs to describe per-endpoint blocking and point users to a TaskNotes-rendered view for constraint- aware display.
- composeDependencyGap: reject a sub-1 value (was composing a "P0D" no-op) and an absurd value that would serialize in scientific notation; the lag input gains step/max so fractional entry is prevented at the source. - StatusManager.isStarted: treat a completed status as started via isCompletedStatus, robust to a status config that was not normalized. - Extract deriveBlockingState so the TaskManager and Bases assembly sites share one per-endpoint derivation instead of duplicating it verbatim. - A broken-link (unresolved) blocker now holds the card pessimistically blocked rather than reading as a satisfied "Dependencies" relationship. - Note the last-wins reltype for a hand-authored duplicate edge. Tests: blocking-edge modify detection in the edit-change diff, reverse accessors dropping a released successor, gap edge cases, and the unresolved-blocker state. Changelog: flag that a custom in-progress status must be categorized Started to release start-based dependencies.
…ders The blocked-by and blocking relationship lists tracked their expanded state only in the DOM, so any card re-render (a related task update, a view refresh) collapsed them with nothing to restore. Move that state into ExpandedProjectsService alongside the subtasks expansion and re-open the list on render when the service says it was open.
… types Non-gating guidance so users configure statuses for the feature to work: - A status-readiness summary under Advanced Dependencies in Features shows the per-category counts and whether start-based (start-to-start / start-to-finish) dependencies can release, since those need a Started status to release before completion. - An inline advisory in the status settings flags a missing category while advanced types are on, at the point statuses are edited. Enablement is never blocked: a degraded state is safe (start-based edges just fall back to releasing on completion), so this guides rather than gates.
When a task gates a successor's finish, the blocking toggle tooltip now reads "Blocking N tasks: X to start, Y to finish" instead of a flat count, mirroring the per-endpoint blocked-by badge. A finish-to-start-only case (no finish-gated successors) keeps the plain summary.
Obsidian tooltips render with white-space: pre-wrap, so put the start and finish totals each on their own line for readability.
…ncy quick path Eight plain-language menu titles (four types x blocking/blocked-by) for the context-menu type-before-task step: readable arrow names plus a direction-specific clause, no raw FS/SS/FF/SF codes.
…u quick path When advanced dependency types are enabled, Dependencies -> Blocking / Blocked by lists the four relationship types before the task picker, and the edge is created with the chosen type. The shared task picker is reused unchanged and toggle-off behavior is identical to before. The reltype-aware add is extracted into taskContextMenuDependencies.ts so the threading is unit-tested.
…methods The card now calls isBlockingExpanded / isBlockedByExpanded / setBlockingExpanded / setBlockedByExpanded (added with expansion persistence in ee33f38), but these two suites still mocked only isExpanded/toggle, so they threw on CI and failed the coverage gate. Complete the mocks to match the service.
… module addReltypeMenuItems lived on TaskContextMenu where the Obsidian Menu made it awkward to unit-test, leaving the new menu-wiring below the new-code coverage bar. Move it into taskContextMenuDependencies.ts behind a minimal Menu seam and cover the type ordering, side-specific titles, and reltype threading.
…lazy build getStartBlockingPredecessorPaths / getFinishBlockingPredecessorPaths and the build-indexes-on-first-query guard were unexercised, holding the branch's new-code coverage under the 80% gate. Add two DependencyCache cases so they run.
The blocked-by / blocking sub-list was rebuilt (clear + refetch + re-render) on every card render via restoreRelationshipExpansion, so a Bases TaskList / Kanban card recreated on EVERY_TASK_UPDATED re-fired the async expansion and oscillated open/closed on frontmatter edits. Expansion is now DOM-only: built when the user expands, not restored on later renders (matching main's behavior).
…stence With the render-time restore gone, ExpandedProjectsService's blocked/blocking expansion tracking had only dead reads and write-only setters. Remove the two sets, their four accessors, and the rename/clear plumbing, plus the setter call sites in the toggle handlers. Blocked/blocking expansion is DOM-only now, so the handlers no longer need a plugin reference. Deletes the obsolete service test.
Activating a Bases leaf re-fires the onDataUpdated lifecycle with unchanged data, and the base scheduled a full re-render (empty the items container and recreate every card) unconditionally. That tore down a card's expanded blocked-by / blocking sub-list on a mere click or focus change, since the expansion is DOM-only and not restored on re-render. Gate the re-render on a data signature the same way CalendarView already does: when the signature is unchanged since the last render, skip. Implemented on the base so both TaskListView and KanbanView are covered.
…sing The readiness row and the enable-time modal both need to name the absent category, which a boolean readiness check cannot carry. Scoped to Not started and Started; a missing Completed category is a separate concern.
Tab content is cached and revealed by class toggle, so a tab the user has already visited shows a DOM built before whatever prompted the navigation. The new entry point clears and re-renders instead; click and keyboard navigation keep the cheaper cache-and-reveal path.
Turning on advanced dependency types with no Started status silently degrades start-anchored relationships to releasing on completion. The modal names the missing category and offers a route to the fix, so the acknowledgement is not a dead end. Resolves a three-way choice rather than a boolean, and treats dismissal as going back.
The toggle now asks before taking effect when a start category has no status, and each answer lands distinctly: going back leaves it off, enabling commits, and the repair path commits then opens the statuses tab. Obsidian flips the toggle before the handler runs, so every path rebuilds the tab from the stored value; the repair path empties it instead, so returning shows the repaired state rather than the warning captured on the way out.
The row's only signal was a left border on the setting item, which Obsidian's native settings group card suppresses, so the warning never rendered. The colour now rides on the row's own text with a decorative glyph, and the copy drops to the counts plus what is missing — the consequence and the remedy are the modal's job. Removes the two border rules that never applied.
…edited The advisory rendered inside a card that starts collapsed, so it was invisible on arrival — including when the enable-time modal had just sent the user here to read it. It now sits above the card, gains a ready state and a link back to the dependency settings, and repaints from the category dropdown's existing change handler so assigning the category clears it without rebuilding the list.
…surfaces current Consolidates the category-label mapping that had grown three copies, so a badge key rename can no longer silently break one readiness surface while the others keep working, and stops the Features row tallying the status set twice per render. Also makes tab invalidation symmetric. The readiness state is shown on two tabs but changed from either, and the settings pane reveals cached tab content on a click, so a change made on one tab left the other asserting a state that was no longer true -- in the worst case the statuses-tab indicator never appeared at all after enabling the feature from Features. Each surface now discards the other's cached content when it changes what that surface reports. The repair path also waits for the destination to take over before discarding the pane it came from, so a refused navigation cannot leave both blank.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d41df08d9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+354
to
+357
| categorySelect.addEventListener("change", () => { | ||
| const value = categorySelect.value as StatusCategory; | ||
| status.category = value; | ||
| status.isCompleted = value === "completed"; |
There was a problem hiding this comment.
When a user follows the migration guidance and changes an existing status from Not started to Started, this handler only saves the mutated status configuration. Although SettingsLifecycleService.applySettingsSideEffects() updates StatusManager, it does not call dependencyCache.updateConfig() because customStatuses is absent from its cache-settings snapshot, so startedStatusByPath retains the old values. Existing start-to-start and start-to-finish edges therefore remain blocked until the predecessor note changes or the plugin reloads; invalidate/rebuild the dependency cache when lifecycle categories change.
Useful? React with 👍 / 👎.
Comment on lines
+196
to
+198
| const dataSignature = this.computeDataUpdateSignature(); | ||
| if (dataSignature !== null && dataSignature === this.lastDataUpdateSignature) { | ||
| return; |
There was a problem hiding this comment.
When a dependency predecessor outside the current Bases result is edited directly in its note, the dependency cache can release or apply an edge without changing any row included in computeDataUpdateSignature(). The metadata-cache task event is ignored as an untracked path, and this equality return then suppresses the Bases refresh, leaving the visible dependent's start/finish badge stale until another tracked row property changes. The signature or invalidation path needs to incorporate dependency-cache lifecycle state for rendered tasks.
Useful? React with 👍 / 👎.
…rized The dependency cache memoizes started/finished per file at index time, and the cache-settings snapshot that gates its rebuild covers taskTag, folders, indexing and field mapping — not the status set. Assigning a Started category therefore refreshed StatusManager while leaving every already-indexed verdict stale, so start-to-start and start-to-finish edges stayed blocked until the predecessor note changed or the plugin reloaded. That is exactly the repair the readiness modal sends the user to make. Track the classification inputs the cache actually reads — a status's value, completed flag and category — and rebuild the dependency indexes when they change. The task cache keeps its own narrower trigger, so recolouring or renaming a status no longer implies a filter-index rebuild and a data-changed broadcast.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34218b7fa4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private getFileRelationshipSignature(path: string): string { | ||
| const blockingTasks = this.sortedSetValues(this.dependencySources.get(path)); | ||
| const blockedTasks = this.sortedSetValues(this.activeDependencyTargets.get(path)); | ||
| const { start, finish } = this.computeBlockedDependents(path); |
There was a problem hiding this comment.
When a task's existing dependency changes only from a start-gating type to a finish-gating type (for example, finish-to-start to finish-to-finish), blockingTasks remains identical and this reverse lookup describes successors of the edited task, not the edited task's own constraining predecessors. The before/after signatures therefore match, so EVENT_DEPENDENCY_CACHE_CHANGED is not emitted; cross-file consumers such as an already-open predecessor relationship widget remain stale because their own note received no metadata event. Include the task's start/finish predecessor sets or its edge reltypes in this signature.
Useful? React with 👍 / 👎.
Comment on lines
+2877
to
+2878
| consequence: | ||
| "Start-to-start and start-to-finish dependencies will release only when the predecessor completes, not when it starts.", |
There was a problem hiding this comment.
When only the Not started (planned) category is missing, every non-completed status is categorized as Started, so start-to-start and start-to-finish edges release immediately rather than only when the predecessor completes. The modal nevertheless always presents this completion-only consequence, which can mislead users deciding whether to enable the feature; select consequence text based on which category is missing.
Useful? React with 👍 / 👎.
…egory The modal carried one consequence line for both trigger conditions, and it was wrong for one of them. Because countStatusCategories normalizes every status into exactly one bucket, no Not started category means every status is either Started or Completed — so isStarted answers true for all of them and start-based edges release immediately rather than only on completion. The modal told that user the opposite of what would happen, at the moment they were deciding whether to enable the feature. Split the string in two and pick by which category is absent: a missing Started still warns that start-based edges wait for completion, while a missing Not started warns that they stop gating altogether. Both missing lands on the same warning, since every remaining status is Completed and therefore started. Also clears two analyzer findings on this work: the modal's injected members are readonly, and the navigation test asserts an empty child list with toHaveLength.
|
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.
Enabling Advanced dependency types while the configured statuses have no Not started or no Started category silently degrades start-anchored relationships: start-to-start and start-to-finish stop releasing when a predecessor starts and release only when it completes. Nothing told you. The existing readiness row tried to, but its only signal was a left border on the setting item, which Obsidian's native settings-group card suppresses — so it rendered nothing at all.
What changes
Go back,Just enable, orEnable and go to statuses.... It re-evaluates the status set on every enable and stores no acknowledgement.Notes for review
tn-dependency-readiness--ok/--warnCSS rules are deleted. They never rendered, and dead CSS that looks like it styles the warning invites the next reader to change its colour rather than its mechanism.switchTabkeeps its cache-and-reveal path for ordinary click and keyboard navigation; only programmatic navigation and explicit invalidation force a rebuild.CI
The suite is red on this branch for 17 pre-existing failures unrelated to this work —
dateUtils, ICS export/duration, pomodoro projected-end-time, mini-calendar sidebar, and overdue/scheduled grouping. All are date-boundary sensitive and fail identically on the base branch with none of these commits applied. They need fixing separately.This branch is stacked on
feat/rfc-status-lifecycle(PR #19) becausemainhas noStatusCategoryat all — that dependency is real, not stacking convenience. The diff againstmaintherefore carries #19's commits alongside these seven.Not verified automatically
Warning visibility inside Obsidian's settings card, in light and dark themes, needs a dogfood build — jsdom cannot prove it.