From bd90c95739d88a249291296aba2dfb2b29680981 Mon Sep 17 00:00:00 2001 From: James Ross Date: Tue, 10 Feb 2026 21:16:04 -0800 Subject: [PATCH 1/9] feat(views): declarative view engine + 4 PRISM views (#189) 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). --- CHANGELOG.md | 7 +- src/index.js | 2 +- src/views.js | 321 ++++++++++++++++++++++++++++++++++++++++----- test/views.test.js | 201 ++++++++++++++++++++++++++-- 4 files changed, 488 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 804d1b6a..b7b0d299 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`getNode()` returns full node info** — ID, extracted prefix, prefix classification (`canonical`/`system`/`unknown`), and properties from the materialized graph - **`git mind nodes` command** — List and inspect nodes with `--prefix ` filtering, `--id ` single-node detail, and `--json` output - **Node formatting** — `formatNode()` and `formatNodeList()` in `src/cli/format.js` for terminal display +- **Declarative view engine** — `declareView(name, config)` compiles prefix/type filter configs into views; existing `roadmap`, `architecture`, `backlog` views refactored to declarative configs +- **`milestone` view** — progress tracking per milestone: child task counts, completion percentage, blockers +- **`traceability` view** — spec-to-implementation gap analysis: identifies unimplemented specs/ADRs, reports coverage percentage +- **`blockers` view** — transitive blocking chain resolution with cycle detection, root blocker identification +- **`onboarding` view** — topologically-sorted reading order for doc/spec/ADR nodes with cycle detection - **Schema validators** — `src/validators.js` enforces GRAPH_SCHEMA.md at runtime: node ID grammar (`prefix:identifier`), edge type validation, confidence type safety (rejects NaN/Infinity/strings), self-edge rejection for `blocks` and `depends-on`, prefix classification with warnings for unknown prefixes - **Validator exports** — `validateNodeId`, `validateEdgeType`, `validateConfidence`, `validateEdge`, `extractPrefix`, `classifyPrefix`, plus constants `NODE_ID_REGEX`, `NODE_ID_MAX_LENGTH`, `CANONICAL_PREFIXES`, `SYSTEM_PREFIXES` @@ -20,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`createEdge()` now validates all inputs** — Node IDs must use `prefix:identifier` format, confidence must be a finite number, self-edges rejected for blocking types - **`EDGE_TYPES` canonical source** moved to `validators.js` (re-exported from `edges.js` for backwards compatibility) -- **Test count** — 87 tests across 6 files (was 74) +- **Test count** — 102 tests across 6 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/index.js b/src/index.js index 359dbe57..6122b96e 100644 --- a/src/index.js +++ b/src/index.js @@ -11,5 +11,5 @@ export { extractPrefix, classifyPrefix, NODE_ID_REGEX, NODE_ID_MAX_LENGTH, CANONICAL_PREFIXES, SYSTEM_PREFIXES, ALL_PREFIXES, } from './validators.js'; -export { defineView, renderView, listViews } from './views.js'; +export { defineView, declareView, renderView, listViews } from './views.js'; export { parseDirectives, processCommit } from './hooks.js'; diff --git a/src/views.js b/src/views.js index 03536d73..4e8b5919 100644 --- a/src/views.js +++ b/src/views.js @@ -1,20 +1,46 @@ /** * @module views - * Observer view definitions and rendering for git-mind. - * Views are filtered projections of the graph. + * Declarative view engine for git-mind. + * Views are filtered, computed projections of the graph. */ +import { extractPrefix } from './validators.js'; + /** @type {Map} */ const registry = new Map(); /** * @typedef {object} ViewDefinition * @property {string} name - * @property {(nodes: string[], edges: Array<{from: string, to: string, label: string, props: object}>) => {nodes: string[], edges: Array<{from: string, to: string, label: string, props: object}>}} filterFn + * @property {string} [description] + * @property {(nodes: string[], edges: Edge[]) => ViewResult} filterFn + */ + +/** + * @typedef {object} Edge + * @property {string} from + * @property {string} to + * @property {string} label + * @property {Record} [props] */ /** - * Register a named view. + * @typedef {object} ViewResult + * @property {string[]} nodes + * @property {Edge[]} edges + * @property {Record} [meta] - Optional computed metadata + */ + +/** + * @typedef {object} ViewConfig + * @property {string[]} prefixes - Node prefix filter + * @property {string[]} [edgeTypes] - Edge type filter (default: all) + * @property {boolean} [requireBothEndpoints=false] - If true, both edge endpoints must be in filtered nodes + * @property {string} [description] + */ + +/** + * Register a named view with a filter function. * * @param {string} name * @param {ViewDefinition['filterFn']} filterFn @@ -23,12 +49,43 @@ export function defineView(name, filterFn) { registry.set(name, { name, filterFn }); } +/** + * Register a declarative view from a config object. + * Compiles the config into a filter function. + * + * @param {string} name + * @param {ViewConfig} config + */ +export function declareView(name, config) { + const prefixSet = new Set(config.prefixes); + const typeSet = config.edgeTypes ? new Set(config.edgeTypes) : null; + const bothEndpoints = config.requireBothEndpoints ?? false; + + const filterFn = (nodes, edges) => { + const matchedNodes = nodes.filter(n => { + const prefix = extractPrefix(n); + return prefix && prefixSet.has(prefix); + }); + const nodeSet = new Set(matchedNodes); + + const matchedEdges = edges.filter(e => { + if (typeSet && !typeSet.has(e.label)) return false; + if (bothEndpoints) return nodeSet.has(e.from) && nodeSet.has(e.to); + return nodeSet.has(e.from) || nodeSet.has(e.to); + }); + + return { nodes: matchedNodes, edges: matchedEdges }; + }; + + registry.set(name, { name, description: config.description, filterFn }); +} + /** * Render a named view against the graph. * * @param {import('@git-stunts/git-warp').default} graph * @param {string} viewName - * @returns {Promise<{nodes: string[], edges: Array<{from: string, to: string, label: string, props: object}>}>} + * @returns {Promise} */ export async function renderView(graph, viewName) { const view = registry.get(viewName); @@ -51,39 +108,28 @@ export function listViews() { return [...registry.keys()]; } -// --- Built-in views --- +// ── Built-in declarative views ────────────────────────────────── -defineView('roadmap', (nodes, edges) => { - // Nodes that look like phases or tasks (by ID prefix convention) - const phaseOrTask = nodes.filter(n => n.startsWith('phase:') || n.startsWith('task:')); - const relevantEdges = edges.filter( - e => phaseOrTask.includes(e.from) || phaseOrTask.includes(e.to) - ); - return { nodes: phaseOrTask, edges: relevantEdges }; +declareView('roadmap', { + description: 'Phase and task nodes with all related edges', + prefixes: ['phase', 'task'], }); -defineView('architecture', (nodes, edges) => { - // Nodes that look like crates/modules and depends-on edges - const modules = nodes.filter(n => - n.startsWith('crate:') || n.startsWith('module:') || n.startsWith('pkg:') - ); - const depEdges = edges.filter( - e => e.label === 'depends-on' && modules.includes(e.from) && modules.includes(e.to) - ); - return { nodes: modules, edges: depEdges }; +declareView('architecture', { + description: 'Module nodes with depends-on edges', + prefixes: ['crate', 'module', 'pkg'], + edgeTypes: ['depends-on'], + requireBothEndpoints: true, }); -defineView('backlog', (nodes, edges) => { - // All task nodes - const tasks = nodes.filter(n => n.startsWith('task:')); - const taskEdges = edges.filter( - e => tasks.includes(e.from) || tasks.includes(e.to) - ); - return { nodes: tasks, edges: taskEdges }; +declareView('backlog', { + description: 'Task nodes with all related edges', + prefixes: ['task'], }); +// ── Built-in imperative views ─────────────────────────────────── + defineView('suggestions', (nodes, edges) => { - // Edges with low confidence (AI-suggested, not yet reviewed) const lowConfEdges = edges.filter(e => { const conf = e.props?.confidence; return typeof conf === 'number' && conf < 0.5; @@ -98,3 +144,218 @@ defineView('suggestions', (nodes, edges) => { edges: lowConfEdges, }; }); + +// ── PRISM views ───────────────────────────────────────────────── + +defineView('milestone', (nodes, edges) => { + // Milestone progress: find milestones, their child tasks (belongs-to), + // and compute completion stats per milestone. + const milestones = nodes.filter(n => n.startsWith('milestone:')); + const tasks = nodes.filter(n => n.startsWith('task:')); + const features = nodes.filter(n => n.startsWith('feature:')); + const relevant = new Set([...milestones, ...tasks, ...features]); + + const relevantEdges = edges.filter( + e => relevant.has(e.from) || relevant.has(e.to) + ); + + // Compute per-milestone stats + const milestoneStats = {}; + for (const m of milestones) { + // Tasks that belong-to this milestone + const children = edges + .filter(e => e.label === 'belongs-to' && e.to === m) + .map(e => e.from); + + // A task is "done" if it has at least one 'implements' edge pointing from it + const done = children.filter(child => + edges.some(e => e.from === child && e.label === 'implements') + ); + + // Blockers: tasks that block children of this milestone + const childSet = new Set(children); + const blockers = edges + .filter(e => e.label === 'blocks' && childSet.has(e.to)) + .map(e => e.from); + + milestoneStats[m] = { + total: children.length, + done: done.length, + pct: children.length > 0 ? Math.round((done.length / children.length) * 100) : 0, + blockers: [...new Set(blockers)], + }; + } + + return { + nodes: [...relevant], + edges: relevantEdges, + meta: { milestoneStats }, + }; +}); + +defineView('traceability', (nodes, edges) => { + // Spec-to-implementation gap analysis. + // Find specs, then check which have 'implements' edges pointing at them. + const specs = nodes.filter(n => n.startsWith('spec:') || n.startsWith('adr:')); + const implementsEdges = edges.filter(e => e.label === 'implements'); + + const implemented = new Set(implementsEdges.map(e => e.to)); + const gaps = specs.filter(s => !implemented.has(s)); + const covered = specs.filter(s => implemented.has(s)); + + // Include all implements edges + the spec/impl nodes + const implNodes = new Set(specs); + for (const e of implementsEdges) { + if (specs.includes(e.to)) { + implNodes.add(e.from); + } + } + + const relevantEdges = implementsEdges.filter(e => specs.includes(e.to)); + + return { + nodes: [...implNodes], + edges: relevantEdges, + meta: { + gaps, + covered, + coveragePct: specs.length > 0 ? Math.round((covered.length / specs.length) * 100) : 100, + }, + }; +}); + +defineView('blockers', (nodes, edges) => { + // Transitive blocking chain resolution with cycle detection. + const blockEdges = edges.filter(e => e.label === 'blocks'); + + // Build adjacency list: blocker -> [blocked] + const adj = new Map(); + for (const e of blockEdges) { + if (!adj.has(e.from)) adj.set(e.from, []); + adj.get(e.from).push(e.to); + } + + // Find all transitive chains from each root blocker + const chains = []; + const cycles = []; + const allInvolved = new Set(); + + // DFS from each node that has outgoing blocks edges + for (const root of adj.keys()) { + const visited = new Set(); + const path = []; + + const dfs = (node) => { + if (path.includes(node)) { + // Cycle detected + const cycleStart = path.indexOf(node); + cycles.push([...path.slice(cycleStart), node]); + return; + } + if (visited.has(node)) return; + visited.add(node); + path.push(node); + allInvolved.add(node); + + const targets = adj.get(node) || []; + for (const t of targets) { + allInvolved.add(t); + dfs(t); + } + path.pop(); + }; + + dfs(root); + } + + // Build chains: root blockers (nodes that block others but aren't blocked themselves) + const blocked = new Set(blockEdges.map(e => e.to)); + const roots = [...adj.keys()].filter(n => !blocked.has(n)); + + for (const root of roots) { + const chain = []; + const buildChain = (node, depth) => { + chain.push({ node, depth }); + for (const t of (adj.get(node) || [])) { + buildChain(t, depth + 1); + } + }; + buildChain(root, 0); + if (chain.length > 1) chains.push(chain); + } + + return { + nodes: [...allInvolved], + edges: blockEdges, + meta: { chains, cycles, rootBlockers: roots }, + }; +}); + +defineView('onboarding', (nodes, edges) => { + // Topologically-sorted reading order for new engineers. + // Uses doc/spec/adr nodes, ordered by dependency edges. + const docNodes = nodes.filter(n => + n.startsWith('doc:') || n.startsWith('spec:') || n.startsWith('adr:') + ); + const docSet = new Set(docNodes); + + // Relevant edges: depends-on, implements, documents between doc nodes + // or pointing to doc nodes + const relevantTypes = new Set(['depends-on', 'documents', 'implements']); + const docEdges = edges.filter(e => + relevantTypes.has(e.label) && (docSet.has(e.from) || docSet.has(e.to)) + ); + + // Build dependency graph for topological sort + // An edge A --[depends-on]--> B means read B before A + // An edge A --[documents]--> B means read A alongside B (no strict order) + const inDegree = new Map(); + const adj = new Map(); + for (const n of docNodes) { + inDegree.set(n, 0); + adj.set(n, []); + } + + for (const e of edges) { + // Only consider depends-on between doc nodes for ordering + if (e.label === 'depends-on' && docSet.has(e.from) && docSet.has(e.to)) { + adj.get(e.to).push(e.from); // B should come before A + inDegree.set(e.from, (inDegree.get(e.from) || 0) + 1); + } + } + + // Kahn's algorithm + const queue = []; + for (const [node, deg] of inDegree) { + if (deg === 0) queue.push(node); + } + queue.sort(); // deterministic tie-break: alphabetical + + const sorted = []; + while (queue.length > 0) { + const node = queue.shift(); + sorted.push(node); + for (const next of (adj.get(node) || [])) { + const newDeg = inDegree.get(next) - 1; + inDegree.set(next, newDeg); + if (newDeg === 0) { + // Insert sorted to maintain deterministic order + const idx = queue.findIndex(q => q > next); + if (idx === -1) queue.push(next); + else queue.splice(idx, 0, next); + } + } + } + + // Nodes not reached (part of a cycle) go at the end + const remaining = docNodes.filter(n => !sorted.includes(n)).sort(); + + return { + nodes: [...sorted, ...remaining], + edges: docEdges, + meta: { + readingOrder: [...sorted, ...remaining], + hasCycles: remaining.length > 0, + }, + }; +}); diff --git a/test/views.test.js b/test/views.test.js index e174e5fa..b970e68e 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -5,7 +5,7 @@ import { tmpdir } from 'node:os'; import { execSync } from 'node:child_process'; import { initGraph } from '../src/graph.js'; import { createEdge } from '../src/edges.js'; -import { renderView, listViews, defineView } from '../src/views.js'; +import { renderView, listViews, defineView, declareView } from '../src/views.js'; describe('views', () => { let tempDir; @@ -21,18 +21,56 @@ describe('views', () => { await rm(tempDir, { recursive: true, force: true }); }); - it('listViews returns built-in views', () => { + // ── Core engine ─────────────────────────────────────────────── + + it('listViews returns all built-in views', () => { const views = listViews(); expect(views).toContain('roadmap'); expect(views).toContain('architecture'); expect(views).toContain('backlog'); expect(views).toContain('suggestions'); + expect(views).toContain('milestone'); + expect(views).toContain('traceability'); + expect(views).toContain('blockers'); + expect(views).toContain('onboarding'); }); it('renderView throws for unknown views', async () => { await expect(renderView(graph, 'nonexistent')).rejects.toThrow(/Unknown view/); }); + it('defineView registers a custom imperative view', async () => { + defineView('custom-test', (nodes, edges) => ({ + nodes: nodes.filter(n => n.startsWith('x:')), + edges: [], + })); + + await createEdge(graph, { source: 'x:foo', target: 'y:bar', type: 'relates-to' }); + + const result = await renderView(graph, 'custom-test'); + expect(result.nodes).toEqual(['x:foo']); + expect(result.edges).toEqual([]); + }); + + it('declareView registers a config-based view', async () => { + declareView('declared-test', { + prefixes: ['pkg'], + edgeTypes: ['depends-on'], + requireBothEndpoints: true, + }); + + await createEdge(graph, { source: 'pkg:a', target: 'pkg:b', type: 'depends-on' }); + await createEdge(graph, { source: 'pkg:a', target: 'task:c', type: 'implements' }); + + const result = await renderView(graph, 'declared-test'); + expect(result.nodes).toContain('pkg:a'); + expect(result.nodes).toContain('pkg:b'); + expect(result.edges.length).toBe(1); + expect(result.edges[0].label).toBe('depends-on'); + }); + + // ── Existing views (now declarative) ────────────────────────── + it('roadmap view filters for phase/task nodes', async () => { await createEdge(graph, { source: 'phase:alpha', target: 'task:build-cli', type: 'blocks' }); await createEdge(graph, { source: 'file:src/main.js', target: 'doc:readme', type: 'documents' }); @@ -54,6 +92,16 @@ describe('views', () => { expect(result.edges[0].label).toBe('depends-on'); }); + 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'); + }); + it('suggestions view filters for low-confidence edges', async () => { await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'relates-to', confidence: 0.3 }); await createEdge(graph, { source: 'task:c', target: 'task:d', type: 'implements', confidence: 1.0 }); @@ -66,16 +114,147 @@ describe('views', () => { expect(result.nodes).not.toContain('task:c'); }); - it('defineView registers a custom view', async () => { - defineView('custom', (nodes, edges) => ({ - nodes: nodes.filter(n => n.startsWith('x:')), - edges: [], - })); + // ── PRISM: milestone view ───────────────────────────────────── - await createEdge(graph, { source: 'x:foo', target: 'y:bar', type: 'relates-to' }); + describe('milestone view', () => { + it('shows milestones with completion stats', async () => { + await createEdge(graph, { source: 'task:a', target: 'milestone:M1', type: 'belongs-to' }); + await createEdge(graph, { source: 'task:b', target: 'milestone:M1', type: 'belongs-to' }); + // task:a implements something (counts as done) + await createEdge(graph, { source: 'task:a', target: 'spec:x', type: 'implements' }); - const result = await renderView(graph, 'custom'); - expect(result.nodes).toEqual(['x:foo']); - expect(result.edges).toEqual([]); + const result = await renderView(graph, 'milestone'); + expect(result.nodes).toContain('milestone:M1'); + expect(result.nodes).toContain('task:a'); + expect(result.nodes).toContain('task:b'); + expect(result.meta.milestoneStats['milestone:M1'].total).toBe(2); + expect(result.meta.milestoneStats['milestone:M1'].done).toBe(1); + expect(result.meta.milestoneStats['milestone:M1'].pct).toBe(50); + }); + + it('reports blockers for a milestone', async () => { + await createEdge(graph, { source: 'task:a', target: 'milestone:M1', type: 'belongs-to' }); + await createEdge(graph, { source: 'task:blocker', target: 'task:a', type: 'blocks' }); + + const result = await renderView(graph, 'milestone'); + expect(result.meta.milestoneStats['milestone:M1'].blockers).toContain('task:blocker'); + }); + + it('handles milestone with no tasks', async () => { + // Create a milestone node by linking it to something + await createEdge(graph, { source: 'milestone:empty', target: 'spec:x', type: 'relates-to' }); + + const result = await renderView(graph, 'milestone'); + expect(result.meta.milestoneStats['milestone:empty'].total).toBe(0); + expect(result.meta.milestoneStats['milestone:empty'].pct).toBe(0); + }); + }); + + // ── PRISM: traceability view ────────────────────────────────── + + describe('traceability view', () => { + it('identifies unimplemented specs as gaps', async () => { + await createEdge(graph, { source: 'spec:auth', target: 'doc:readme', type: 'documents' }); + await createEdge(graph, { source: 'spec:session', target: 'doc:readme', type: 'documents' }); + await createEdge(graph, { source: 'file:auth.js', target: 'spec:auth', type: 'implements' }); + + const result = await renderView(graph, 'traceability'); + expect(result.meta.gaps).toContain('spec:session'); + expect(result.meta.gaps).not.toContain('spec:auth'); + expect(result.meta.covered).toContain('spec:auth'); + expect(result.meta.coveragePct).toBe(50); + }); + + it('reports 100% when all specs are implemented', async () => { + await createEdge(graph, { source: 'file:a.js', target: 'spec:auth', type: 'implements' }); + + const result = await renderView(graph, 'traceability'); + expect(result.meta.gaps).toEqual([]); + expect(result.meta.coveragePct).toBe(100); + }); + + it('includes ADRs in traceability', async () => { + await createEdge(graph, { source: 'adr:001', target: 'doc:readme', type: 'documents' }); + + const result = await renderView(graph, 'traceability'); + expect(result.meta.gaps).toContain('adr:001'); + }); + }); + + // ── PRISM: blockers view ────────────────────────────────────── + + describe('blockers view', () => { + it('follows transitive blocking chains', async () => { + await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'blocks' }); + await createEdge(graph, { source: 'task:b', target: 'task:c', type: 'blocks' }); + + const result = await renderView(graph, 'blockers'); + expect(result.nodes).toContain('task:a'); + expect(result.nodes).toContain('task:b'); + expect(result.nodes).toContain('task:c'); + expect(result.meta.rootBlockers).toContain('task:a'); + expect(result.meta.rootBlockers).not.toContain('task:b'); + expect(result.meta.chains.length).toBe(1); + expect(result.meta.chains[0].length).toBe(3); + }); + + it('detects cycles', async () => { + await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'blocks' }); + await createEdge(graph, { source: 'task:b', target: 'task:a', type: 'blocks' }); + + const result = await renderView(graph, 'blockers'); + expect(result.meta.cycles.length).toBeGreaterThan(0); + }); + + it('returns empty for graph with no blocks edges', async () => { + await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'relates-to' }); + + const result = await renderView(graph, 'blockers'); + expect(result.nodes).toEqual([]); + expect(result.edges).toEqual([]); + expect(result.meta.chains).toEqual([]); + expect(result.meta.cycles).toEqual([]); + }); + }); + + // ── PRISM: onboarding view ──────────────────────────────────── + + describe('onboarding view', () => { + it('returns topologically sorted reading order', async () => { + // spec:basics should come before spec:advanced (advanced depends on basics) + await createEdge(graph, { source: 'spec:advanced', target: 'spec:basics', type: 'depends-on' }); + + const result = await renderView(graph, 'onboarding'); + const order = result.meta.readingOrder; + expect(order.indexOf('spec:basics')).toBeLessThan(order.indexOf('spec:advanced')); + }); + + it('includes doc and adr nodes', async () => { + await createEdge(graph, { source: 'doc:guide', target: 'adr:001', type: 'documents' }); + + const result = await renderView(graph, 'onboarding'); + expect(result.nodes).toContain('doc:guide'); + expect(result.nodes).toContain('adr:001'); + }); + + it('handles graphs with no doc nodes', async () => { + await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'blocks' }); + + const result = await renderView(graph, 'onboarding'); + expect(result.nodes).toEqual([]); + expect(result.meta.readingOrder).toEqual([]); + expect(result.meta.hasCycles).toBe(false); + }); + + it('detects cycles in doc dependencies', async () => { + await createEdge(graph, { source: 'spec:a', target: 'spec:b', type: 'depends-on' }); + await createEdge(graph, { source: 'spec:b', target: 'spec:a', type: 'depends-on' }); + + const result = await renderView(graph, 'onboarding'); + expect(result.meta.hasCycles).toBe(true); + // Both nodes should still appear + expect(result.nodes).toContain('spec:a'); + expect(result.nodes).toContain('spec:b'); + }); }); }); From 1893fa7f4c4f82bd26a6260d73d236f11f28834e Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 06:58:11 -0800 Subject: [PATCH 2/9] fix(views): address PR #190 review feedback (#189) - 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 --- CHANGELOG.md | 5 ++++- src/views.js | 18 ++++++++++++------ test/views.test.js | 20 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0484a7ac..baa4592f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`buildChain` stack overflow on cyclic graphs** — Root blocker leading into a cycle (e.g., `C → A → B → A`) caused infinite recursion; added visited guard (#189) +- **Duplicate cycle reports in blockers view** — Per-root DFS visited sets caused the same cycle to be reported from multiple entry points; switched to global visited set (#189) +- **O(n*m) lookups in traceability/onboarding views** — Replaced `Array.includes()` with `Set.has()` for spec and sorted membership checks (#189) - **YAML arrays now rejected by `parseImportFile`** — `typeof [] === 'object'` no longer bypasses the guard; arrays produce "not an object" error instead of a confusing "Missing version" (#187) - **Array-typed `node.properties` rejected during validation** — `validateImportData` now rejects arrays in `properties`, preventing `Object.entries` from writing numeric-indexed keys (#187) - **Edge `createdAt` renamed to `importedAt`** — The timestamp on imported edges now honestly reflects import time; avoids misleading "creation" semantics on re-import (#187) @@ -40,7 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`isLowConfidence()` shared helper** — Low-confidence threshold (`< 0.5`) extracted from `status.js` and `views.js` into `validators.js` to keep the threshold in one place (#185) - **`createEdge()` now validates all inputs** — Node IDs must use `prefix:identifier` format, confidence must be a finite number, self-edges rejected for blocking types - **`EDGE_TYPES` canonical source** moved to `validators.js` (re-exported from `edges.js` for backwards compatibility) -- **Test count** — 136 tests across 8 files (was 74) +- **Test count** — 138 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/views.js b/src/views.js index 73829e59..8e649739 100644 --- a/src/views.js +++ b/src/views.js @@ -146,8 +146,8 @@ defineView('suggestions', (nodes, edges) => { // ── PRISM views ───────────────────────────────────────────────── defineView('milestone', (nodes, edges) => { - // Milestone progress: find milestones, their child tasks (belongs-to), - // and compute completion stats per milestone. + // Milestone progress: find milestones and their children (tasks + features + // linked via belongs-to), then compute completion stats per milestone. const milestones = nodes.filter(n => n.startsWith('milestone:')); const tasks = nodes.filter(n => n.startsWith('task:')); const features = nodes.filter(n => n.startsWith('feature:')); @@ -195,6 +195,7 @@ defineView('traceability', (nodes, edges) => { // Spec-to-implementation gap analysis. // Find specs, then check which have 'implements' edges pointing at them. const specs = nodes.filter(n => n.startsWith('spec:') || n.startsWith('adr:')); + const specSet = new Set(specs); const implementsEdges = edges.filter(e => e.label === 'implements'); const implemented = new Set(implementsEdges.map(e => e.to)); @@ -204,12 +205,12 @@ defineView('traceability', (nodes, edges) => { // Include all implements edges + the spec/impl nodes const implNodes = new Set(specs); for (const e of implementsEdges) { - if (specs.includes(e.to)) { + if (specSet.has(e.to)) { implNodes.add(e.from); } } - const relevantEdges = implementsEdges.filter(e => specs.includes(e.to)); + const relevantEdges = implementsEdges.filter(e => specSet.has(e.to)); return { nodes: [...implNodes], @@ -239,8 +240,8 @@ defineView('blockers', (nodes, edges) => { const allInvolved = new Set(); // DFS from each node that has outgoing blocks edges + const visited = new Set(); for (const root of adj.keys()) { - const visited = new Set(); const path = []; const dfs = (node) => { @@ -272,7 +273,10 @@ defineView('blockers', (nodes, edges) => { for (const root of roots) { const chain = []; + const seen = new Set(); const buildChain = (node, depth) => { + if (seen.has(node)) return; + seen.add(node); chain.push({ node, depth }); for (const t of (adj.get(node) || [])) { buildChain(t, depth + 1); @@ -330,9 +334,11 @@ defineView('onboarding', (nodes, edges) => { queue.sort(); // deterministic tie-break: alphabetical const sorted = []; + const sortedSet = new Set(); while (queue.length > 0) { const node = queue.shift(); sorted.push(node); + sortedSet.add(node); for (const next of (adj.get(node) || [])) { const newDeg = inDegree.get(next) - 1; inDegree.set(next, newDeg); @@ -346,7 +352,7 @@ defineView('onboarding', (nodes, edges) => { } // Nodes not reached (part of a cycle) go at the end - const remaining = docNodes.filter(n => !sorted.includes(n)).sort(); + const remaining = docNodes.filter(n => !sortedSet.has(n)).sort(); return { nodes: [...sorted, ...remaining], diff --git a/test/views.test.js b/test/views.test.js index b970e68e..1564ea40 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -140,6 +140,16 @@ describe('views', () => { expect(result.meta.milestoneStats['milestone:M1'].blockers).toContain('task:blocker'); }); + it('includes features in milestone stats', async () => { + await createEdge(graph, { source: 'task:a', target: 'milestone:M1', type: 'belongs-to' }); + await createEdge(graph, { source: 'feature:login', target: 'milestone:M1', type: 'belongs-to' }); + await createEdge(graph, { source: 'feature:login', target: 'spec:auth', type: 'implements' }); + + const result = await renderView(graph, 'milestone'); + expect(result.meta.milestoneStats['milestone:M1'].total).toBe(2); + expect(result.meta.milestoneStats['milestone:M1'].done).toBe(1); + }); + it('handles milestone with no tasks', async () => { // Create a milestone node by linking it to something await createEdge(graph, { source: 'milestone:empty', target: 'spec:x', type: 'relates-to' }); @@ -206,6 +216,16 @@ describe('views', () => { expect(result.meta.cycles.length).toBeGreaterThan(0); }); + it('handles a root blocker leading into a cycle', async () => { + await createEdge(graph, { source: 'task:root', target: 'task:a', type: 'blocks' }); + await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'blocks' }); + await createEdge(graph, { source: 'task:b', target: 'task:a', type: 'blocks' }); + + const result = await renderView(graph, 'blockers'); + expect(result.meta.rootBlockers).toContain('task:root'); + expect(result.meta.cycles.length).toBeGreaterThan(0); + }); + it('returns empty for graph with no blocks edges', async () => { await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'relates-to' }); From c816f15642785a0003477b4b3242b48957fa9241 Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 08:38:58 -0800 Subject: [PATCH 3/9] fix(views): milestone children filter and DFS on-stack optimization (#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 --- CHANGELOG.md | 2 +- src/views.js | 12 ++++++++---- test/views.test.js | 9 +++++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index baa4592f..4e7b0b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`isLowConfidence()` shared helper** — Low-confidence threshold (`< 0.5`) extracted from `status.js` and `views.js` into `validators.js` to keep the threshold in one place (#185) - **`createEdge()` now validates all inputs** — Node IDs must use `prefix:identifier` format, confidence must be a finite number, self-edges rejected for blocking types - **`EDGE_TYPES` canonical source** moved to `validators.js` (re-exported from `edges.js` for backwards compatibility) -- **Test count** — 138 tests across 8 files (was 74) +- **Test count** — 139 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/views.js b/src/views.js index 8e649739..c9412170 100644 --- a/src/views.js +++ b/src/views.js @@ -160,12 +160,13 @@ defineView('milestone', (nodes, edges) => { // Compute per-milestone stats const milestoneStats = {}; for (const m of milestones) { - // Tasks that belong-to this milestone + // Tasks and features that belong-to this milestone const children = edges - .filter(e => e.label === 'belongs-to' && e.to === m) + .filter(e => e.label === 'belongs-to' && e.to === m + && (e.from.startsWith('task:') || e.from.startsWith('feature:'))) .map(e => e.from); - // A task is "done" if it has at least one 'implements' edge pointing from it + // A child is "done" if it has at least one 'implements' edge pointing from it const done = children.filter(child => edges.some(e => e.from === child && e.label === 'implements') ); @@ -243,9 +244,10 @@ defineView('blockers', (nodes, edges) => { const visited = new Set(); for (const root of adj.keys()) { const path = []; + const onStack = new Set(); const dfs = (node) => { - if (path.includes(node)) { + if (onStack.has(node)) { // Cycle detected const cycleStart = path.indexOf(node); cycles.push([...path.slice(cycleStart), node]); @@ -254,6 +256,7 @@ defineView('blockers', (nodes, edges) => { if (visited.has(node)) return; visited.add(node); path.push(node); + onStack.add(node); allInvolved.add(node); const targets = adj.get(node) || []; @@ -262,6 +265,7 @@ defineView('blockers', (nodes, edges) => { dfs(t); } path.pop(); + onStack.delete(node); }; dfs(root); diff --git a/test/views.test.js b/test/views.test.js index 1564ea40..05c579bc 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -150,6 +150,15 @@ describe('views', () => { expect(result.meta.milestoneStats['milestone:M1'].done).toBe(1); }); + it('excludes non-task/non-feature children from milestone stats', async () => { + await createEdge(graph, { source: 'task:a', target: 'milestone:M1', type: 'belongs-to' }); + await createEdge(graph, { source: 'spec:loose', target: 'milestone:M1', type: 'belongs-to' }); + + const result = await renderView(graph, 'milestone'); + // spec:loose should not count as a milestone child + expect(result.meta.milestoneStats['milestone:M1'].total).toBe(1); + }); + it('handles milestone with no tasks', async () => { // Create a milestone node by linking it to something await createEdge(graph, { source: 'milestone:empty', target: 'spec:x', type: 'relates-to' }); From 6df0d71aa0e4116aef5bb357fb3224de4c108eff Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 08:47:54 -0800 Subject: [PATCH 4/9] fix(views): address PR #190 R2/R3 review feedback (#189) - 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 --- CHANGELOG.md | 3 ++- src/index.js | 2 +- src/views.js | 25 ++++++++++++++++++++++--- test/views.test.js | 21 ++++++++++++++++----- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e7b0b6b..16447fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`isLowConfidence()` shared helper** — Low-confidence threshold (`< 0.5`) extracted from `status.js` and `views.js` into `validators.js` to keep the threshold in one place (#185) - **`createEdge()` now validates all inputs** — Node IDs must use `prefix:identifier` format, confidence must be a finite number, self-edges rejected for blocking types - **`EDGE_TYPES` canonical source** moved to `validators.js` (re-exported from `edges.js` for backwards compatibility) -- **Test count** — 139 tests across 8 files (was 74) +- **`resetViews()` for test cleanup** — Removes test-registered views from the module-level registry, restoring built-in-only state (#189) +- **Test count** — 140 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/index.js b/src/index.js index e64062c6..c1349f9f 100644 --- a/src/index.js +++ b/src/index.js @@ -14,5 +14,5 @@ export { NODE_ID_REGEX, NODE_ID_MAX_LENGTH, CANONICAL_PREFIXES, SYSTEM_PREFIXES, ALL_PREFIXES, LOW_CONFIDENCE_THRESHOLD, } from './validators.js'; -export { defineView, declareView, renderView, listViews } from './views.js'; +export { defineView, declareView, renderView, listViews, resetViews } from './views.js'; export { parseDirectives, processCommit } from './hooks.js'; diff --git a/src/views.js b/src/views.js index c9412170..03919807 100644 --- a/src/views.js +++ b/src/views.js @@ -108,6 +108,19 @@ export function listViews() { return [...registry.keys()]; } +/** @type {Set} Built-in view names, captured after registration */ +let builtInNames; + +/** + * Remove all views that were not registered at module load time. + * Intended for test cleanup. + */ +export function resetViews() { + for (const name of registry.keys()) { + if (!builtInNames.has(name)) registry.delete(name); + } +} + // ── Built-in declarative views ────────────────────────────────── declareView('roadmap', { @@ -157,6 +170,11 @@ defineView('milestone', (nodes, edges) => { e => relevant.has(e.from) || relevant.has(e.to) ); + // Pre-compute set of nodes that have an outgoing 'implements' edge + const hasImplements = new Set( + edges.filter(e => e.label === 'implements').map(e => e.from) + ); + // Compute per-milestone stats const milestoneStats = {}; for (const m of milestones) { @@ -167,9 +185,7 @@ defineView('milestone', (nodes, edges) => { .map(e => e.from); // A child is "done" if it has at least one 'implements' edge pointing from it - const done = children.filter(child => - edges.some(e => e.from === child && e.label === 'implements') - ); + const done = children.filter(child => hasImplements.has(child)); // Blockers: tasks that block children of this milestone const childSet = new Set(children); @@ -367,3 +383,6 @@ defineView('onboarding', (nodes, edges) => { }, }; }); + +// Capture built-in names after all registrations +builtInNames = new Set(registry.keys()); diff --git a/test/views.test.js b/test/views.test.js index 05c579bc..1d67f737 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -5,7 +5,7 @@ import { tmpdir } from 'node:os'; import { execSync } from 'node:child_process'; import { initGraph } from '../src/graph.js'; import { createEdge } from '../src/edges.js'; -import { renderView, listViews, defineView, declareView } from '../src/views.js'; +import { renderView, listViews, defineView, declareView, resetViews } from '../src/views.js'; describe('views', () => { let tempDir; @@ -18,6 +18,7 @@ describe('views', () => { }); afterEach(async () => { + resetViews(); await rm(tempDir, { recursive: true, force: true }); }); @@ -198,6 +199,14 @@ describe('views', () => { const result = await renderView(graph, 'traceability'); expect(result.meta.gaps).toContain('adr:001'); }); + + it('reports 100% coverage when no specs or ADRs exist', async () => { + await createEdge(graph, { source: 'task:a', target: 'task:b', type: 'blocks' }); + + const result = await renderView(graph, 'traceability'); + expect(result.meta.gaps).toEqual([]); + expect(result.meta.coveragePct).toBe(100); + }); }); // ── PRISM: blockers view ────────────────────────────────────── @@ -249,13 +258,15 @@ describe('views', () => { // ── PRISM: onboarding view ──────────────────────────────────── describe('onboarding view', () => { - it('returns topologically sorted reading order', async () => { - // spec:basics should come before spec:advanced (advanced depends on basics) - await createEdge(graph, { source: 'spec:advanced', target: 'spec:basics', type: 'depends-on' }); + it('returns topologically sorted reading order for a 3-node chain', async () => { + // spec:a -> spec:b -> spec:c (c depends on b, b depends on a) + await createEdge(graph, { source: 'spec:b', target: 'spec:a', type: 'depends-on' }); + await createEdge(graph, { source: 'spec:c', target: 'spec:b', type: 'depends-on' }); const result = await renderView(graph, 'onboarding'); const order = result.meta.readingOrder; - expect(order.indexOf('spec:basics')).toBeLessThan(order.indexOf('spec:advanced')); + expect(order.indexOf('spec:a')).toBeLessThan(order.indexOf('spec:b')); + expect(order.indexOf('spec:b')).toBeLessThan(order.indexOf('spec:c')); }); it('includes doc and adr nodes', async () => { From 87460688694f0ace22d3b592b0b90d74781dc4e6 Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 09:00:04 -0800 Subject: [PATCH 5/9] fix(views): address PR #190 R4 review feedback (#189) - 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 --- src/views.js | 4 ++-- test/views.test.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/views.js b/src/views.js index 03919807..c299502e 100644 --- a/src/views.js +++ b/src/views.js @@ -109,7 +109,7 @@ export function listViews() { } /** @type {Set} Built-in view names, captured after registration */ -let builtInNames; +let builtInNames = new Set(); /** * Remove all views that were not registered at module load time. @@ -342,7 +342,7 @@ defineView('onboarding', (nodes, edges) => { // Only consider depends-on between doc nodes for ordering if (e.label === 'depends-on' && docSet.has(e.from) && docSet.has(e.to)) { adj.get(e.to).push(e.from); // B should come before A - inDegree.set(e.from, (inDegree.get(e.from) || 0) + 1); + inDegree.set(e.from, inDegree.get(e.from) + 1); } } diff --git a/test/views.test.js b/test/views.test.js index 1d67f737..c612069f 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -121,7 +121,7 @@ describe('views', () => { it('shows milestones with completion stats', async () => { await createEdge(graph, { source: 'task:a', target: 'milestone:M1', type: 'belongs-to' }); await createEdge(graph, { source: 'task:b', target: 'milestone:M1', type: 'belongs-to' }); - // task:a implements something (counts as done) + // Completion heuristic: a child is "done" if it has an outgoing 'implements' edge await createEdge(graph, { source: 'task:a', target: 'spec:x', type: 'implements' }); const result = await renderView(graph, 'milestone'); @@ -167,6 +167,7 @@ describe('views', () => { const result = await renderView(graph, 'milestone'); expect(result.meta.milestoneStats['milestone:empty'].total).toBe(0); expect(result.meta.milestoneStats['milestone:empty'].pct).toBe(0); + expect(result.meta.milestoneStats['milestone:empty'].blockers).toEqual([]); }); }); From 55f6f0844f185ef46889c110bebdeb3d8c4cfcdb Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 09:00:50 -0800 Subject: [PATCH 6/9] docs(changelog): add R4 review fixes (#189) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16447fbb..1185f454 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`createEdge()` now validates all inputs** — Node IDs must use `prefix:identifier` format, confidence must be a finite number, self-edges rejected for blocking types - **`EDGE_TYPES` canonical source** moved to `validators.js` (re-exported from `edges.js` for backwards compatibility) - **`resetViews()` for test cleanup** — Removes test-registered views from the module-level registry, restoring built-in-only state (#189) +- **`builtInNames` initialized defensively** — Prevents `TypeError` if `resetViews()` is called before module finishes init (#189) +- **Removed dead `|| 0` fallback in onboarding view** — `inDegree` map is pre-initialized for all doc nodes, so the guard was unreachable (#189) - **Test count** — 140 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 From cd61492523765d7b080864f7b6857ecc323e5eee Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 10:21:38 -0800 Subject: [PATCH 7/9] fix(views): address PR #190 R5 review feedback (#189) - 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() --- CHANGELOG.md | 3 ++- src/views.js | 3 ++- test/views.test.js | 13 +++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1185f454..5cff9477 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`resetViews()` for test cleanup** — Removes test-registered views from the module-level registry, restoring built-in-only state (#189) - **`builtInNames` initialized defensively** — Prevents `TypeError` if `resetViews()` is called before module finishes init (#189) - **Removed dead `|| 0` fallback in onboarding view** — `inDegree` map is pre-initialized for all doc nodes, so the guard was unreachable (#189) -- **Test count** — 140 tests across 8 files (was 74) +- **Milestone view returns self-contained subgraph** — Edge filter tightened from `||` to `&&` so returned edges only reference nodes in the result; eliminates dangling `implements` references to spec nodes (#189) +- **Test count** — 141 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/views.js b/src/views.js index c299502e..e1f89c32 100644 --- a/src/views.js +++ b/src/views.js @@ -116,6 +116,7 @@ let builtInNames = new Set(); * Intended for test cleanup. */ export function resetViews() { + // Safe: ES Map iterators handle deletion of not-yet-visited entries for (const name of registry.keys()) { if (!builtInNames.has(name)) registry.delete(name); } @@ -167,7 +168,7 @@ defineView('milestone', (nodes, edges) => { const relevant = new Set([...milestones, ...tasks, ...features]); const relevantEdges = edges.filter( - e => relevant.has(e.from) || relevant.has(e.to) + e => relevant.has(e.from) && relevant.has(e.to) ); // Pre-compute set of nodes that have an outgoing 'implements' edge diff --git a/test/views.test.js b/test/views.test.js index c612069f..9bc0b7ec 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -160,6 +160,19 @@ describe('views', () => { expect(result.meta.milestoneStats['milestone:M1'].total).toBe(1); }); + it('returns edges as a self-contained subgraph', async () => { + await createEdge(graph, { source: 'task:a', target: 'milestone:M1', type: 'belongs-to' }); + // implements edge targets spec:x which is NOT a milestone/task/feature + await createEdge(graph, { source: 'task:a', target: 'spec:x', type: 'implements' }); + + const result = await renderView(graph, 'milestone'); + const nodeSet = new Set(result.nodes); + for (const e of result.edges) { + expect(nodeSet.has(e.from)).toBe(true); + expect(nodeSet.has(e.to)).toBe(true); + } + }); + it('handles milestone with no tasks', async () => { // Create a milestone node by linking it to something await createEdge(graph, { source: 'milestone:empty', target: 'spec:x', type: 'relates-to' }); From 97c2321e72a8d5b5028f6a039b751fe4e3df1cfa Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 13:30:45 -0800 Subject: [PATCH 8/9] fix(views): onboarding view self-contained subgraph (#189) - 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 --- CHANGELOG.md | 3 ++- src/views.js | 2 +- test/views.test.js | 13 +++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cff9477..2787bcd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`builtInNames` initialized defensively** — Prevents `TypeError` if `resetViews()` is called before module finishes init (#189) - **Removed dead `|| 0` fallback in onboarding view** — `inDegree` map is pre-initialized for all doc nodes, so the guard was unreachable (#189) - **Milestone view returns self-contained subgraph** — Edge filter tightened from `||` to `&&` so returned edges only reference nodes in the result; eliminates dangling `implements` references to spec nodes (#189) -- **Test count** — 141 tests across 8 files (was 74) +- **Onboarding view returns self-contained subgraph** — Same `||` → `&&` fix applied to `docEdges` filter; prevents non-doc nodes (e.g., `file:`) from appearing as dangling edge endpoints (#189) +- **Test count** — 142 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/views.js b/src/views.js index e1f89c32..ec26b344 100644 --- a/src/views.js +++ b/src/views.js @@ -326,7 +326,7 @@ defineView('onboarding', (nodes, edges) => { // or pointing to doc nodes const relevantTypes = new Set(['depends-on', 'documents', 'implements']); const docEdges = edges.filter(e => - relevantTypes.has(e.label) && (docSet.has(e.from) || docSet.has(e.to)) + relevantTypes.has(e.label) && docSet.has(e.from) && docSet.has(e.to) ); // Build dependency graph for topological sort diff --git a/test/views.test.js b/test/views.test.js index 9bc0b7ec..673990e4 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -300,6 +300,19 @@ describe('views', () => { expect(result.meta.hasCycles).toBe(false); }); + it('returns edges as a self-contained subgraph', async () => { + await createEdge(graph, { source: 'spec:a', target: 'spec:b', type: 'depends-on' }); + // implements edge from a non-doc node into a doc node + await createEdge(graph, { source: 'file:auth.js', target: 'spec:a', type: 'implements' }); + + const result = await renderView(graph, 'onboarding'); + const nodeSet = new Set(result.nodes); + for (const e of result.edges) { + expect(nodeSet.has(e.from)).toBe(true); + expect(nodeSet.has(e.to)).toBe(true); + } + }); + it('detects cycles in doc dependencies', async () => { await createEdge(graph, { source: 'spec:a', target: 'spec:b', type: 'depends-on' }); await createEdge(graph, { source: 'spec:b', target: 'spec:a', type: 'depends-on' }); From 137dfbd227a1baf74a3eb43e56a05040d60b5a13 Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 15:25:02 -0800 Subject: [PATCH 9/9] fix(views): address PR #190 R7 review feedback (#189) - 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 --- CHANGELOG.md | 5 ++++- src/views.js | 36 ++++++++++++++++++++++++++---------- test/views.test.js | 5 +++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2787bcd6..351d5395 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Removed dead `|| 0` fallback in onboarding view** — `inDegree` map is pre-initialized for all doc nodes, so the guard was unreachable (#189) - **Milestone view returns self-contained subgraph** — Edge filter tightened from `||` to `&&` so returned edges only reference nodes in the result; eliminates dangling `implements` references to spec nodes (#189) - **Onboarding view returns self-contained subgraph** — Same `||` → `&&` fix applied to `docEdges` filter; prevents non-doc nodes (e.g., `file:`) from appearing as dangling edge endpoints (#189) -- **Test count** — 142 tests across 8 files (was 74) +- **`declareView` validates `config.prefixes`** — Throws on missing or empty prefixes array, surfacing misconfiguration early (#189) +- **Milestone view O(M×E) → O(E+M) edge lookups** — Pre-indexes `belongs-to` and `blocks` edges by target before the milestone loop (#189) +- **Onboarding ordering loop uses pre-filtered `docEdges`** — Eliminates redundant `docSet.has()` checks in dependency graph construction (#189) +- **Test count** — 143 tests across 8 files (was 74) ## [2.0.0-alpha.0] - 2026-02-07 diff --git a/src/views.js b/src/views.js index ec26b344..f8586421 100644 --- a/src/views.js +++ b/src/views.js @@ -57,6 +57,9 @@ export function defineView(name, filterFn) { * @param {ViewConfig} config */ export function declareView(name, config) { + if (!Array.isArray(config.prefixes) || config.prefixes.length === 0) { + throw new Error(`declareView("${name}"): config.prefixes must be a non-empty array`); + } const prefixSet = new Set(config.prefixes); const typeSet = config.edgeTypes ? new Set(config.edgeTypes) : null; const bothEndpoints = config.requireBothEndpoints ?? false; @@ -176,23 +179,37 @@ defineView('milestone', (nodes, edges) => { edges.filter(e => e.label === 'implements').map(e => e.from) ); + // Pre-index belongs-to and blocks edges by target for O(E + M) lookups + const belongsToByTarget = new Map(); + const blocksByTarget = new Map(); + for (const e of edges) { + if (e.label === 'belongs-to') { + if (!belongsToByTarget.has(e.to)) belongsToByTarget.set(e.to, []); + belongsToByTarget.get(e.to).push(e); + } else if (e.label === 'blocks') { + if (!blocksByTarget.has(e.to)) blocksByTarget.set(e.to, []); + blocksByTarget.get(e.to).push(e); + } + } + // Compute per-milestone stats const milestoneStats = {}; for (const m of milestones) { // Tasks and features that belong-to this milestone - const children = edges - .filter(e => e.label === 'belongs-to' && e.to === m - && (e.from.startsWith('task:') || e.from.startsWith('feature:'))) + const children = (belongsToByTarget.get(m) || []) + .filter(e => e.from.startsWith('task:') || e.from.startsWith('feature:')) .map(e => e.from); // A child is "done" if it has at least one 'implements' edge pointing from it const done = children.filter(child => hasImplements.has(child)); // Blockers: tasks that block children of this milestone - const childSet = new Set(children); - const blockers = edges - .filter(e => e.label === 'blocks' && childSet.has(e.to)) - .map(e => e.from); + const blockers = []; + for (const child of children) { + for (const e of (blocksByTarget.get(child) || [])) { + blockers.push(e.from); + } + } milestoneStats[m] = { total: children.length, @@ -339,9 +356,8 @@ defineView('onboarding', (nodes, edges) => { adj.set(n, []); } - for (const e of edges) { - // Only consider depends-on between doc nodes for ordering - if (e.label === 'depends-on' && docSet.has(e.from) && docSet.has(e.to)) { + for (const e of docEdges) { + if (e.label === 'depends-on') { adj.get(e.to).push(e.from); // B should come before A inDegree.set(e.from, inDegree.get(e.from) + 1); } diff --git a/test/views.test.js b/test/views.test.js index 673990e4..6aa94b70 100644 --- a/test/views.test.js +++ b/test/views.test.js @@ -53,6 +53,11 @@ describe('views', () => { expect(result.edges).toEqual([]); }); + it('declareView throws on missing or empty prefixes', () => { + expect(() => declareView('bad-view', {})).toThrow('prefixes must be a non-empty array'); + expect(() => declareView('bad-view', { prefixes: [] })).toThrow('prefixes must be a non-empty array'); + }); + it('declareView registers a config-based view', async () => { declareView('declared-test', { prefixes: ['pkg'],