feat: declarative view engine + 4 PRISM views#190
Merged
flyingrobots merged 10 commits intomainfrom Feb 11, 2026
Merged
Conversation
Adds declareView() for config-based views, refactors roadmap/architecture/ backlog to declarative configs. Implements 4 new views: milestone (progress tracking), traceability (spec gap analysis), blockers (transitive chains with cycle detection), onboarding (topological sort). 21 view tests (102 total).
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a declarative view engine: new APIs Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Registry as ViewRegistry
participant Compiler as ConfigCompiler
participant Engine as ViewsEngine
participant Graph as GraphStore
rect rgba(100,149,237,0.5)
Client->>Registry: declareView(name, config)
Registry->>Compiler: compile(config)
Compiler-->>Registry: filterFn + meta
Registry-->>Client: confirmation (registered)
end
rect rgba(60,179,113,0.5)
Client->>Engine: renderView(graph, viewName)
Engine->>Registry: lookup viewName → filterFn
Engine->>Graph: fetch nodes & edges
Engine->>Engine: run filterFn(edges) → ViewResult
Engine-->>Client: Promise<ViewResult>
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
# Conflicts: # CHANGELOG.md # src/views.js
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 242-267: The current loop creates a fresh visited set for each
root which causes duplicate cycle reports; change to use a single global visited
Set declared outside the for-loop (replace the per-root const visited = new
Set() with one visited declared before iterating adj.keys()) so nodes are only
explored once by dfs(node), or alternatively keep the existing per-root
traversal but deduplicate cycles after collection by normalizing each cycle via
canonical rotation (rotate the cycle so its lexicographically smallest node is
first and compare stringified forms to filter duplicates) before pushing into
cycles; update references to visited, dfs, cycles, path and allInvolved
accordingly.
- Around line 148-192: The code collects features into the relevant set but
never treats them as milestone children; update the children/done/blockers
computation in defineView('milestone') so that e.from nodes that
startWith('feature:') are treated like tasks: when building children use
edges.filter(e => e.label === 'belongs-to' && e.to === m &&
(e.from.startsWith('task:') || e.from.startsWith('feature:'))). Then compute
done by checking implements edges from those children and compute blockers by
checking blocks edges where e.to is in the childSet so milestoneStats (total,
done, pct, blockers) includes features the same as tasks.
- Around line 273-283: The recursive buildChain function can infinite-recurse on
graphs with cycles; add a per-root visited guard to prevent revisiting nodes:
create a Set (e.g., visited) local to the loop over roots, mark a node as
visited when entering buildChain, only recurse into neighbors from adj.get(node)
if they are not in visited, and avoid pushing duplicates into chain; this
preserves the DFS behavior used to populate chain while preventing stack
overflows when cycles exist.
- Around line 194-223: The traceability view builds a specs array and then
repeatedly calls specs.includes(...) (in the defineView('traceability')
function, used when populating implNodes and filtering relevantEdges), causing
O(n·m) behavior; convert specs to a Set (e.g., specsSet = new Set(specs)) and
replace all specs.includes(...) checks with specsSet.has(...), iterate over
specsSet when initializing implNodes if needed, and use the Set when computing
implemented/covered/gaps to maintain correct results while reducing lookup
complexity.
- Around line 346-353: The membership check for building remaining uses
sorted.includes(n) which is O(n) per check causing O(n²) behavior; create a Set
from the sorted array (e.g., const sortedSet = new Set(sorted>) and use
sortedSet.has(n) when filtering docNodes to build remaining, or maintain a Set
in tandem while populating sorted; then compute remaining = docNodes.filter(n =>
!sortedSet.has(n)).sort() and return nodes: [...sorted, ...remaining], edges:
docEdges unchanged.
In `@test/views.test.js`:
- Around line 201-207: Add the missing integration test that creates a root
blocker feeding into a cycle (as described) and asserts that
renderView('blockers') returns the root in result.meta.rootBlockers and reports
cycles; then fix buildChain in src/views.js to avoid infinite recursion by
tracking a visited/stack set per chain (detect if the next node is already on
the current chain, record the cycle and stop recursion for that branch) so
buildChain returns safely when encountering cycles and allows renderView to
populate rootBlockers and meta.cycles correctly.
- Fix buildChain stack overflow on cyclic graphs (Critical) - Use global visited set in DFS to prevent duplicate cycle reports - Use Set for O(1) lookups in traceability and onboarding views - Add test for root blocker leading into a cycle - Add test for features in milestone stats
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 242-268: The DFS uses path.includes(node) which is O(n); add a
parallel Set (e.g., onStack) alongside the path array and update it whenever you
push/pop in dfs to make the on-stack check O(1): use onStack.has(node) instead
of path.includes(node) to detect cycles, but retain path.indexOf(node) for
computing cycleStart and slicing when recording cycles; ensure onStack.add(node)
is called when you push to path and onStack.delete(node) when you pop so
visited, dfs, adj, allInvolved, and cycles semantics remain unchanged.
In `@test/views.test.js`:
- Around line 42-53: Tests are polluting the module-level registry Map because
defineView/declareView mutate it and there's no teardown; fix by adding a
resetViews() export in src/views.js that clears the registry Map (or expose a
clearRegistry function) and call that in the test file's afterEach(), or
alternatively make test view names unique per-test (e.g., include a random
suffix) when calling defineView/declareView to avoid collisions; touch symbols:
registry, defineView, declareView, resetViews (new), and ensure tests call
resetViews() in afterEach() before returning to avoid cross-test leakage.
…189) - Filter milestone children to task:/feature: prefixes only (prevents spec:/doc: nodes from inflating stats via belongs-to edges) - Use parallel Set for O(1) on-stack cycle detection in blockers DFS - Add test for non-task/non-feature exclusion from milestone stats
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/views.js (1)
90-101: 🧹 Nitpick | 🔵 Trivial
renderViewreturn type isPromise<ViewResult>but imperativefilterFns return plainViewResult.This works because
asyncfunctions auto-wrap synchronous returns, but the JSDoc onfilterFn(line 16) declares a synchronous return signature(nodes, edges) => ViewResult. This is fine at runtime, but if a consumer ever writes afilterFnthat returns aPromise<ViewResult>(which the JSDoc onrenderViewwould lead them to expect is supported),renderViewwill return a double-wrapped promise and the result will be the inner promise object rather than the resolved value.Consider either documenting that
filterFnmust be synchronous, or adding anawaiton line 100 to safely handle both sync and async filter functions:Proposed fix
- return view.filterFn(nodes, edges); + return await view.filterFn(nodes, edges);
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 148-193: The milestone view is doing an O(children × edges) check
by calling edges.some(...) for each child; precompute a Set of nodes that have
outgoing 'implements' edges (e.g., build implementsFrom = new Set(...) from
edges where e.label === 'implements') before looping milestones, then replace
the per-child edges.some(...) check in the compute of done with
implementsFrom.has(child); update the logic around
defineView('milestone')/milestoneStats/done to use that Set so the complexity
becomes O(edges + children).
In `@test/views.test.js`:
- Around line 251-288: Add a third node to the "returns topologically sorted
reading order" test to validate transitive ordering: after creating the
depends-on edge between 'spec:advanced' and 'spec:basics' (via createEdge), also
create edges forming a three-node chain (e.g., createEdge with source 'spec:c'
-> target 'spec:b' and createEdge with source 'spec:b' -> target 'spec:a'), then
call renderView(graph, 'onboarding') and assert that readingOrder indexes
reflect spec:a < spec:b < spec:c (and still spec:basics before spec:advanced) so
the topological sort handles transitive dependencies.
- Around line 174-201: Add a new test case in views.test.js for the
"traceability view" that verifies behavior when there are no spec/ADR nodes:
without creating any edges or nodes, call renderView(graph, 'traceability') and
assert result.meta.coveragePct is 100 and result.meta.gaps is an empty array (or
contains no specs/ADRs); reference the existing helpers createEdge and
renderView so the test follows the same pattern as the other cases and ensures
vacuous-truth coverage is enforced.
- Add resetViews() for test cleanup of registry pollution - Pre-compute hasImplements Set in milestone view (O(edges) not O(children*edges)) - Add vacuous-truth traceability test (100% when no specs exist) - Strengthen onboarding test with 3-node transitive chain - Export resetViews from public API
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 341-347: The inDegree increment in the for loop over edges uses a
redundant fallback: replace the expression in the block that currently does
inDegree.set(e.from, (inDegree.get(e.from) || 0) + 1) with a direct increment
using the existing initialized value (e.g. inDegree.set(e.from,
inDegree.get(e.from) + 1)); this affects the loop that inspects edges and the
variables edges, docSet, adj, and inDegree and assumes inDegree was initialized
for all doc nodes earlier.
- Around line 111-122: builtInNames may be undefined when resetViews() runs,
causing builtInNames.has to throw; either initialize builtInNames to an empty
Set at declaration or defensively guard inside resetViews() (e.g., treat missing
builtInNames as an empty set) so that resetViews() iterates registry.keys()
safely; update the declaration of builtInNames or add a check in resetViews()
referencing builtInNames, resetViews, and registry accordingly.
In `@test/views.test.js`:
- Around line 163-170: Add an assertion to the "handles milestone with no tasks"
test to verify that blockers is an empty array for the milestone with no tasks;
locate the test that calls createEdge(...) and renderView(graph, 'milestone')
and add an expect on result.meta.milestoneStats['milestone:empty'].blockers
toEqual([]) so the test validates the blockers aggregation for empty milestones.
- Around line 121-134: The test currently assumes a task is "done" only when
there is an implements edge (created via createEdge) and verifies milestone
stats from renderView (meta.milestoneStats); make this explicit or centralize
the heuristic: either add a short inline comment in the test explaining the
assumption that implements is the sole completion signal, or better refactor the
completion check into a single function (e.g., isTaskComplete or
taskCompletionHeuristic) used by renderView and referenced in tests so future
edge types (e.g., completes or status-based completion) can be added in one
place and tests updated accordingly.
- Initialize builtInNames defensively as new Set() - Remove dead || 0 fallback on pre-initialized inDegree map - Document completion heuristic in milestone test - Assert empty blockers on no-tasks milestone
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 161-209: The returned edges (relevantEdges) can reference nodes
not included in the returned nodes array ([...relevant]); to fix, make the
result a self-contained subgraph by filtering relevantEdges to only include
edges whose both endpoints are in the relevant set (use the existing relevant
Set), or alternatively expand the returned nodes to include every unique
endpoint found in relevantEdges; update the logic inside defineView('milestone',
...) around the relevantEdges computation and the final return so edges and
nodes are consistent.
- Around line 118-122: The loop in resetViews deletes entries from registry
while iterating registry.keys(), which is safe per ES Map iteration semantics
but subtle; add an inline comment inside resetViews (near the for...of and
references to registry and builtInNames) stating that deleting not-yet-visited
Map entries during iteration is defined and intentional, and mention why
builtInNames are skipped so future maintainers aren’t surprised.
- Milestone view edge filter requires both endpoints in node set, preventing dangling references to spec/doc nodes - Add regression test for self-contained subgraph invariant - Add inline comment documenting Map deletion safety in resetViews()
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 326-330: The docEdges set currently includes edges where only one
endpoint is in docSet which can create dangling references in result.edges;
update the docEdges filtering (used with relevantTypes and docSet) to use the
same requireBothEndpoints pattern as the milestone view so that edges are only
kept when both e.from and e.to are in docSet (or alternatively, if intended, add
the external endpoints to result.nodes instead of filtering: collect the non-doc
endpoints referenced by docEdges and append them to result.nodes); modify the
logic around docEdges (and any downstream use of result.nodes/result.edges) to
ensure consistency between edges and nodes.
- Tighten docEdges filter from || to && so returned edges only reference doc/spec/adr nodes present in result.nodes - Add regression test for self-contained subgraph invariant
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 162-210: The per-milestone computation in defineView('milestone',
...) is doing O(M×E) scans because you call edges.filter(...) inside the loop to
find belongs-to children and blocks; refactor by pre-indexing edges by
label+target before the for (const m of milestones) loop — build Maps (e.g.,
belongsToMap: Map<target, string[]> and blocksMap: Map<target, string[]>) using
edges once (similar to how hasImplements is derived), then inside the milestone
loop look up children = belongsToMap.get(m) and blockers = blocksMap.get(child)
(or collect blockers by checking each child against blocksMap) and update
milestoneStats accordingly to reduce complexity to O(E + M).
- Around line 59-81: The declareView function currently accepts missing/empty
config.prefixes which results in an empty Set and a view that matches nothing;
update declareView to validate config.prefixes before creating prefixSet: ensure
config.prefixes is defined, is an array (or iterable) and not empty, and if the
check fails throw a clear Error (or return early) mentioning the view name;
reference the symbols declareView, config.prefixes, and prefixSet when adding
this guard so the validation occurs before new Set(config.prefixes) and before
registry.set(name, ...).
- Around line 342-348: The loop over edges re-checks doc membership redundantly;
replace iterating over edges with iterating over the already-computed docEdges
and only handle those with e.label === 'depends-on', removing the
docSet.has(...) checks; update the loop that currently references edges to use
docEdges and keep the same body updating adj and inDegree (symbols: edges,
docEdges, docSet, adj, inDegree, e.label).
- declareView() validates config.prefixes (non-empty array required) - Milestone view pre-indexes belongs-to/blocks edges by target (O(E+M)) - Onboarding ordering loop uses pre-filtered docEdges
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/views.js`:
- Around line 393-401: The return builds identical arrays twice; create a single
shared array (e.g., readingOrder) from [...sorted, ...remaining] and reuse it
for both nodes and meta.readingOrder to avoid duplication and maintain a single
reference; update the return to use that readingOrder for nodes and
meta.readingOrder, and keep hasCycles computed from remaining.length and edges
as before (ensure you do not mutate the shared readingOrder).
In `@test/views.test.js`:
- Around line 101-109: The test for backlog view currently asserts nodes only;
update it to also assert edges include the 'blocks' edge between task:a and
task:b and exclude the 'depends-on' edge between module:x and module:y. After
calling renderView(graph, 'backlog'), add assertions against result.edges (or
whatever shape renderView returns) to verify an edge for source 'task:a' target
'task:b' with type 'blocks' is present and no edge with source 'module:x' target
'module:y' (type 'depends-on') is present; use the existing createEdge/test
helpers and the result from renderView to locate and assert those edges.
Comment on lines
+393
to
+401
|
|
||
| return { | ||
| nodes: [...sorted, ...remaining], | ||
| edges: docEdges, | ||
| meta: { | ||
| readingOrder: [...sorted, ...remaining], | ||
| hasCycles: remaining.length > 0, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
nodes and readingOrder in the return value are identical arrays — consider deduplicating.
nodes (line 395) and meta.readingOrder (line 398) are both [...sorted, ...remaining]. They carry the same content but are two separate array spreads. If the intent is for consumers to always use meta.readingOrder for the ordered list and nodes for generic subgraph membership, this duplication is fine semantically — but you could assign once and reuse:
♻️ Suggested simplification
+ const readingOrder = [...sorted, ...remaining];
return {
- nodes: [...sorted, ...remaining],
+ nodes: readingOrder,
edges: docEdges,
meta: {
- readingOrder: [...sorted, ...remaining],
+ readingOrder,
hasCycles: remaining.length > 0,
},
};🤖 Prompt for AI Agents
In `@src/views.js` around lines 393 - 401, The return builds identical arrays
twice; create a single shared array (e.g., readingOrder) from [...sorted,
...remaining] and reuse it for both nodes and meta.readingOrder to avoid
duplication and maintain a single reference; update the return to use that
readingOrder for nodes and meta.readingOrder, and keep hasCycles computed from
remaining.length and edges as before (ensure you do not mutate the shared
readingOrder).
Comment on lines
+101
to
+109
| it('backlog view filters for task nodes', async () => { | ||
| await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'blocks' }); | ||
| await createEdge(graph, { source: 'module:x', target: 'module:y', type: 'depends-on' }); | ||
|
|
||
| const result = await renderView(graph, 'backlog'); | ||
| expect(result.nodes).toContain('task:a'); | ||
| expect(result.nodes).toContain('task:b'); | ||
| expect(result.nodes).not.toContain('module:x'); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider asserting that backlog edges are included.
The backlog test verifies node filtering but doesn't check result.edges. Since backlog uses the default OR endpoint matching (not requireBothEndpoints), the blocks edge between task:a and task:b should be included, and it could be worth asserting that, along with the absence of the depends-on edge between module:x and module:y.
🤖 Prompt for AI Agents
In `@test/views.test.js` around lines 101 - 109, The test for backlog view
currently asserts nodes only; update it to also assert edges include the
'blocks' edge between task:a and task:b and exclude the 'depends-on' edge
between module:x and module:y. After calling renderView(graph, 'backlog'), add
assertions against result.edges (or whatever shape renderView returns) to verify
an edge for source 'task:a' target 'task:b' with type 'blocks' is present and no
edge with source 'module:x' target 'module:y' (type 'depends-on') is present;
use the existing createEdge/test helpers and the result from renderView to
locate and assert those edges.
This was referenced Feb 12, 2026
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
declareView(name, config)compiles prefix/type filter configs into viewsroadmap,architecture,backlogfrom imperative filter functions to declarative configsimplementsedges? Coverage %View meta
New views return a
metafield alongsidenodesandedges:Files changed
src/views.js— rewritten: declarative engine + 4 new viewssrc/index.js— exportsdeclareViewtest/views.test.js— rewritten: 21 tests (was 6)CHANGELOG.md— updatedTest plan
declareViewcompiles prefix/type/bothEndpoints configsCloses #189
Summary by CodeRabbit
New Features
Documentation
Tests