feat: Work DAG analysis suite + bijou v1.2.0 adoption#33
Merged
flyingrobots merged 11 commits intomainfrom Mar 4, 2026
Merged
Conversation
- Replace userland Kahn's topo sort with graph.traverse.topologicalSort() (P1-01) - Remove unused reverseReachability import (P2-01) - Remove dead allNodes set in computeProvenance (P2-02) - Replace non-null assertions with guard patterns (P3-01/02/04) - Extract DEFAULT_COLORS constant to eliminate redundant assertion (P3-03) - Remove 237KB SVG from tracking, add docs/assets/*.svg to .gitignore (P4-01)
Upgrade @flyingrobots/bijou, bijou-node, bijou-tui from v0.10.0 to v1.2.0. New features: dagPane(), focusArea(), granular port types, detectColorScheme(). No breaking changes — 788/788 tests pass.
Replace ~50 lines of manual dagLayout() + viewport() + scroll-centering math in roadmap-view.ts with bijou v1.2.0's dagPane() building block. - RoadmapState: dagScrollY/dagScrollX → dagPane: DagPaneState | null - Extract buildDagSource() for DagSource adapter construction - Snapshot-loaded handler builds dagPane with critical path highlighting - Selection sync: j/k navigation updates dagPane selection - Scroll handlers delegate to dagPanePageDown/PageUp/ScrollByX - Resize handler recreates dagPane with new dimensions - No-deps fallback uses fixed scrollY: 0
Replace bare viewport() calls in dashboard-view.ts with bijou v1.2.0's focusArea() building block. The focused column gets a bright gutter indicator, the unfocused column gets a muted gutter. Tab switches focus and the gutter color swaps — instant visual indicator of which panel accepts PgDn/PgUp input. No model changes — FocusAreaState is created on the fly from existing leftScrollY/rightScrollY values.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces raw dagScrollX/dagScrollY state and ad-hoc topo sorting with bijou v1.2.0 pane primitives: loadGraph now returns a topologically sorted ID list; DashboardApp and roadmap view use DagPane/DagPaneState and focusArea; scripts/tests updated for guarded access and DEFAULT_COLORS usage. Changes
Sequence Diagram(s)sequenceDiagram
participant DA as DashboardApp
participant Graph as Graph Engine
participant Pane as DagPane
participant Table as Roadmap Table
DA->>Graph: loadGraph(snapshot) → tasks, campaigns, sorted
Graph-->>DA: return tasks, campaigns, sorted
DA->>Graph: computeCriticalPath(sorted, tasks)
Graph-->>DA: critical path set
DA->>Pane: createDagPaneState(buildDagSource(...), size, options)
Pane-->>DA: DagPaneState
alt user focuses table row
Table->>DA: focusRow → questId
DA->>Pane: dagPaneSelectNode(questId)
Pane-->>DA: selection updated
DA->>Table: sync focus state
end
alt user pages/scrolls DAG
DA->>Pane: dagPanePageDown/Up / dagPaneScrollByX
Pane-->>DA: updated DagPaneState
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![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: d103c8f21a
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.gitignore (1)
15-15: Consider narrowing the SVG ignore pattern to generated artifacts only.
docs/assets/*.svgwill ignore all SVGs in that folder, including future hand-maintained diagrams/icons. If only generated DAG outputs should be ignored, a narrower pattern reduces accidental omissions.Example narrow pattern
-docs/assets/*.svg +docs/assets/work-dag*.svg🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 15, The .gitignore currently ignores all SVGs via the pattern docs/assets/*.svg which may hide hand-edited icons; replace or supplement that entry by narrowing it to only generated artifacts (e.g., change docs/assets/*.svg to a more specific pattern matching generated DAG outputs such as docs/assets/*-dag.svg or docs/assets/generated/*.svg) so that hand-maintained SVGs remain tracked; update the docs/assets/*.svg line accordingly and add a comment explaining the pattern and what is considered generated.scripts/generate-work-dag.ts (1)
343-346: Drop the unusedsortedparameter frombuildAnalysisInputs().Line 345 is currently unused, and passing it on Line 639 makes the API look semantically required when it is not.
♻️ Proposed cleanup
-function buildAnalysisInputs( - tasks: Map<string, TaskNode>, - sorted: string[], -): { +function buildAnalysisInputs( + tasks: Map<string, TaskNode>, +): { summaries: TaskSummary[]; edges: DepEdge[]; } { @@ - const { summaries, edges } = buildAnalysisInputs(tasks, sorted); + const { summaries, edges } = buildAnalysisInputs(tasks);Also applies to: 639-639
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-work-dag.ts` around lines 343 - 346, The function buildAnalysisInputs currently accepts an unused parameter named sorted; remove the sorted parameter from buildAnalysisInputs' signature and update all call sites that pass sorted (e.g., the call that currently passes sorted) to stop providing that argument so the API no longer suggests the value is required; ensure you update any related TypeScript types or function usages (function name: buildAnalysisInputs) so compilation succeeds after the parameter is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tui/bijou/DashboardApp.ts`:
- Around line 402-404: The computed pane sizes can become zero or negative on
small terminals; update the width/height calculations in DashboardApp.ts
(symbols: leftWidth, dagWidth, dagHeight and the other analogous calculations
later) to clamp to safe minimums using Math.max so they never go <= 0 (e.g.,
keep leftWidth = Math.max(28, Math.floor(msg.columns * 0.3)), dagWidth =
Math.max(1, msg.columns - leftWidth - 1), dagHeight = Math.max(1, msg.rows -
3)); apply the same defensive Math.max clamping to the alternate calculations
around the second block that uses these same symbols.
- Around line 447-473: The DAG pane is only created when dagEdges.length > 0
which leaves roadmap.dagPane null for snapshots that have tasks but no
dependency edges; change the logic to create a DagPane whenever there are
dagTasks (or roadmapQuestIds) present: compute the criticalPath only if
dagEdges.length > 0 (calling computeCriticalPath when edges exist) but always
call buildDagSource and createDagPaneState when there are tasks, set dagOptions
as before, and still apply dagPaneSelectNode using
selectedId/roadmapQuestIds/newRoadmapTable.focusRow; in short, remove the strict
dagEdges gating around createDagPaneState and instead gate only the
critical-path computation.
---
Nitpick comments:
In @.gitignore:
- Line 15: The .gitignore currently ignores all SVGs via the pattern
docs/assets/*.svg which may hide hand-edited icons; replace or supplement that
entry by narrowing it to only generated artifacts (e.g., change
docs/assets/*.svg to a more specific pattern matching generated DAG outputs such
as docs/assets/*-dag.svg or docs/assets/generated/*.svg) so that hand-maintained
SVGs remain tracked; update the docs/assets/*.svg line accordingly and add a
comment explaining the pattern and what is considered generated.
In `@scripts/generate-work-dag.ts`:
- Around line 343-346: The function buildAnalysisInputs currently accepts an
unused parameter named sorted; remove the sorted parameter from
buildAnalysisInputs' signature and update all call sites that pass sorted (e.g.,
the call that currently passes sorted) to stop providing that argument so the
API no longer suggests the value is required; ensure you update any related
TypeScript types or function usages (function name: buildAnalysisInputs) so
compilation succeeds after the parameter is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e035f5dd-91eb-4abe-8b22-f9aef713d60a
⛔ Files ignored due to path filters (2)
docs/assets/work-dag.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.gitignoreCHANGELOG.mdpackage.jsonscripts/generate-work-dag.tssrc/domain/services/DagAnalysis.tssrc/tui/bijou/DashboardApp.tssrc/tui/bijou/__tests__/DashboardApp.test.tssrc/tui/bijou/__tests__/views.test.tssrc/tui/bijou/views/dashboard-view.tssrc/tui/bijou/views/roadmap-view.tstest/unit/DagAnalysis.test.ts
💤 Files with no reviewable changes (1)
- src/domain/services/DagAnalysis.ts
Owner
Author
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/tui/bijou/DashboardApp.ts (1)
454-462: Consider centralizing snapshot→DepEdge[]projection.Lines 454–462 duplicate dependency-edge extraction logic also present in
src/tui/bijou/views/roadmap-view.ts. A shared helper would reduce drift risk between model-update and rendering paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/bijou/DashboardApp.ts` around lines 454 - 462, Duplicate logic builds dependency edges from a snapshot (creating dagEdges from snap.quests using q.dependsOn) in DashboardApp.ts (dagTasks/dagEdges with snap.quests) and the roadmap-view; extract that projection into a shared helper function (e.g., buildDepEdgesFromSnapshot or projectSnapshotDepEdges) and replace the inline loop in DashboardApp (and the corresponding code in roadmap-view) to call the helper; ensure the helper accepts the same snapshot shape (or an array of quests) and returns DepEdge[] so both rendering and model-update paths use the single implementation.src/tui/bijou/views/roadmap-view.ts (1)
29-31: PrefersortedTaskIdsforSlicedDagSource.ids()ordering.Line 29 uses raw quest insertion order. Using
snap.sortedTaskIds(filtered to known quests) will keep DAG layout/order deterministic across refreshes.💡 Suggested change
- const questIds = snap.quests.map(q => q.id); + const questIds = snap.sortedTaskIds.length > 0 + ? snap.sortedTaskIds.filter((id) => questMap.has(id)) + : snap.quests.map((q) => q.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/bijou/views/roadmap-view.ts` around lines 29 - 31, The current SlicedDagSource.ids() implementation builds questIds from snap.quests (questIds variable) which preserves insertion order; change it to derive ids from snap.sortedTaskIds (filtered to only include IDs that exist in snap.quests) so DAG layout/order is deterministic across refreshes—use snap.sortedTaskIds.filter(id => snap.quests.some(q => q.id === id)) (or build a Set of snap.quests ids for faster lookup) and return that list from ids() instead of questIds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 26-27: The release notes conflict about the .gitignore SVG
pattern—one line claims it was narrowed to `docs/assets/work-dag*.svg` while
another claims `docs/assets/*.svg` was added; reconcile them by deciding the
true change and updating both occurrences in CHANGELOG.md (references: the
`docs/assets/work-dag*.svg` and `docs/assets/*.svg` phrases) so the same pattern
and rationale appear consistently (e.g., change the Line 26 entry to match Line
37 or vice versa) and ensure the note explains why that specific pattern exists.
- Line 32: Update the CHANGELOG entry to remove the contradictory phrase by
clearly stating the direction of the change: replace the “reimplemented in
userland” wording with an unambiguous description such as “Replaced hand-rolled
Kahn’s algorithm with graph.traverse.topologicalSort()” and mention the affected
module (generate-work-dag.ts) so it reads: generate-work-dag.ts now delegates to
graph.traverse.topologicalSort() instead of using a hand-rolled Kahn
implementation.
In `@src/tui/bijou/__tests__/DashboardApp.test.ts`:
- Around line 304-334: The test currently asserts rendered output equality to
verify paging; instead assert the roadmap fallback scroll state: after calling
app.update(makeKey('pagedown'), loaded) verify that the returned model
(afterPgDn) has roadmap.fallbackScrollY increased relative to the initial
loaded. Then after calling app.update(makeKey('pageup'), afterPgDn) verify the
returned model (afterPgUp) has roadmap.fallbackScrollY equal to the original
loaded value (or decreased appropriately). Locate and update assertions around
makeKey('pagedown')/makeKey('pageup'), app.update(), app.view(), and the
DashboardModel to check roadmap.fallbackScrollY delta/reset rather than
comparing full rendered strings.
---
Nitpick comments:
In `@src/tui/bijou/DashboardApp.ts`:
- Around line 454-462: Duplicate logic builds dependency edges from a snapshot
(creating dagEdges from snap.quests using q.dependsOn) in DashboardApp.ts
(dagTasks/dagEdges with snap.quests) and the roadmap-view; extract that
projection into a shared helper function (e.g., buildDepEdgesFromSnapshot or
projectSnapshotDepEdges) and replace the inline loop in DashboardApp (and the
corresponding code in roadmap-view) to call the helper; ensure the helper
accepts the same snapshot shape (or an array of quests) and returns DepEdge[] so
both rendering and model-update paths use the single implementation.
In `@src/tui/bijou/views/roadmap-view.ts`:
- Around line 29-31: The current SlicedDagSource.ids() implementation builds
questIds from snap.quests (questIds variable) which preserves insertion order;
change it to derive ids from snap.sortedTaskIds (filtered to only include IDs
that exist in snap.quests) so DAG layout/order is deterministic across
refreshes—use snap.sortedTaskIds.filter(id => snap.quests.some(q => q.id ===
id)) (or build a Set of snap.quests ids for faster lookup) and return that list
from ids() instead of questIds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c8bc1ce-5667-4123-b6ae-94087685702f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.gitignoreCHANGELOG.mdpackage.jsonscripts/generate-work-dag.tssrc/tui/bijou/DashboardApp.tssrc/tui/bijou/__tests__/DashboardApp.test.tssrc/tui/bijou/__tests__/views.test.tssrc/tui/bijou/views/roadmap-view.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- src/tui/bijou/tests/views.test.ts
Owner
Author
|
Addressed the remaining open review items in commit ae885b8:
Validation:
|
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.
Summary
DagAnalysis.ts) for DAG structure analysis: level assignment, width, greedy worker scheduling, transitive reduction/closure, anti-chain decomposition, reverse reachability, provenance tracing. Generator script (scripts/generate-work-dag.ts) produces SVG visualizations +work.mdanalysis doc.@flyingrobots/bijou,@flyingrobots/bijou-node,@flyingrobots/bijou-tuifrom v0.10.0 to v1.2.0.dagPane()— replaced ~50 lines of manualdagLayout()+viewport()+ scroll-centering in roadmap view with bijou'sdagPane()building block. Auto-scroll-to-selection, keyboard-synced DAG highlight.focusArea()— dashboard columns now show a colored gutter indicating which panel has focus (bright = active, muted = inactive).graph:pull/graph:pushscripts, CI traceability job.Test plan
npm run build— 0 TypeScript errorsnpm run lint— 0 errors, 0 warningsnpm run test:local— 788/788 tests passnpx tsx xyph-dashboard.tsx— roadmap DAG renders with auto-scroll, dashboard columns show focus gutterSummary by CodeRabbit
New Features
Bug Fixes
Chores