From 5adc71219325a96f6d25edd006ca81ecf3430b26 Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Mon, 27 Apr 2026 11:51:57 +0000 Subject: [PATCH] feat(graph/mutation): implement removeTask, removeDependency, updateTask, updateEdgeAttributes --- src/graph/construction.ts | 66 +++++ src/graph/mutation.ts | 98 +++++++- tasks/implementation/graph/mutation.md | 26 +- test/mutation.test.ts | 328 +++++++++++++++++++++++++ 4 files changed, 508 insertions(+), 10 deletions(-) create mode 100644 test/mutation.test.ts diff --git a/src/graph/construction.ts b/src/graph/construction.ts index 8b0f929..3b25b5a 100644 --- a/src/graph/construction.ts +++ b/src/graph/construction.ts @@ -2,6 +2,12 @@ import { DirectedGraph } from 'graphology'; import type { TaskGraphNodeAttributes, TaskGraphEdgeAttributes, TaskGraphSerialized } from '../schema/index.js'; +import { + removeTask as _removeTask, + removeDependency as _removeDependency, + updateTask as _updateTask, + updateEdgeAttributes as _updateEdgeAttributes, +} from './mutation.js'; /** * Internal graph type alias for the graphology DirectedGraph with our attribute types. @@ -79,6 +85,66 @@ export class TaskGraph { return `${source}->${target}`; } + // --------------------------------------------------------------------------- + // Mutation methods + // --------------------------------------------------------------------------- + + /** + * Remove a task (node) from the graph. + * + * No-op if the node doesn't exist. Graphology automatically removes + * all edges attached to the dropped node (cascade edge removal). + * + * @param id - The task ID to remove + */ + removeTask(id: string): void { + _removeTask(this._graph, id); + } + + /** + * Remove a dependency (edge) from the graph. + * + * No-op if the edge doesn't exist. Uses the deterministic edge key + * `${prerequisite}->${dependent}` to identify the edge (per ADR-006). + * + * @param prerequisite - Source (prerequisite) task ID + * @param dependent - Target (dependent) task ID + */ + removeDependency(prerequisite: string, dependent: string): void { + _removeDependency(this._graph, prerequisite, dependent); + } + + /** + * Update a task's attributes with a partial merge. + * + * Throws `TaskNotFoundError` if the task ID doesn't exist. + * Uses a shallow merge of the provided attributes into the existing + * node attributes. + * + * @param id - The task ID to update + * @param attributes - Partial attributes to merge into the existing node + * @throws {TaskNotFoundError} If the task ID doesn't exist in the graph + */ + updateTask(id: string, attributes: Partial): void { + _updateTask(this._graph, id, attributes); + } + + /** + * Update an edge's attributes with a partial merge. + * + * Throws `TaskNotFoundError` if the edge doesn't exist. + * Uses the deterministic edge key `${prerequisite}->${dependent}` to + * identify the edge (per ADR-006). + * + * @param prerequisite - Source (prerequisite) task ID + * @param dependent - Target (dependent) task ID + * @param attrs - Partial edge attributes to merge into the existing edge + * @throws {TaskNotFoundError} If the edge doesn't exist in the graph + */ + updateEdgeAttributes(prerequisite: string, dependent: string, attrs: Partial): void { + _updateEdgeAttributes(this._graph, prerequisite, dependent, attrs); + } + // --------------------------------------------------------------------------- // Static construction methods (stubs — implementation in dependent tasks) // --------------------------------------------------------------------------- diff --git a/src/graph/mutation.ts b/src/graph/mutation.ts index 80e3602..5a34066 100644 --- a/src/graph/mutation.ts +++ b/src/graph/mutation.ts @@ -1 +1,97 @@ -// Graph mutations — removeTask, removeDependency, updateTask, updateEdgeAttributes \ No newline at end of file +// Graph mutations — removeTask, removeDependency, updateTask, updateEdgeAttributes + +import type { TaskGraphInner } from './construction.js'; +import type { TaskGraphNodeAttributes, TaskGraphEdgeAttributes } from '../schema/index.js'; +import { TaskNotFoundError } from '../error/index.js'; + +/** + * Remove a task (node) from the graph. + * + * No-op if the node doesn't exist — removal is idempotent. + * Graphology automatically removes all edges attached to the dropped node + * (cascade edge removal). + * + * @param graph - The underlying graphology DirectedGraph instance + * @param id - The task ID to remove + */ +export function removeTask(graph: TaskGraphInner, id: string): void { + if (!graph.hasNode(id)) { + return; + } + graph.dropNode(id); +} + +/** + * Remove a dependency (edge) from the graph. + * + * No-op if the edge doesn't exist — removal is idempotent. + * Uses the deterministic edge key `${prerequisite}->${dependent}` to + * identify the edge (per ADR-006). + * + * @param graph - The underlying graphology DirectedGraph instance + * @param prerequisite - Source (prerequisite) task ID + * @param dependent - Target (dependent) task ID + */ +export function removeDependency(graph: TaskGraphInner, prerequisite: string, dependent: string): void { + const key = `${prerequisite}->${dependent}`; + if (!graph.hasEdge(key)) { + return; + } + graph.dropEdge(key); +} + +/** + * Update a task's attributes with a partial merge. + * + * Throws `TaskNotFoundError` if the task ID doesn't exist — + * cannot update attributes of a non-existent node. + * + * Uses `mergeNodeAttributes` for a shallow merge of the provided + * attributes into the existing node attributes. + * + * @param graph - The underlying graphology DirectedGraph instance + * @param id - The task ID to update + * @param attributes - Partial attributes to merge into the existing node + * @throws {TaskNotFoundError} If the task ID doesn't exist in the graph + */ +export function updateTask( + graph: TaskGraphInner, + id: string, + attributes: Partial, +): void { + if (!graph.hasNode(id)) { + throw new TaskNotFoundError(id); + } + graph.mergeNodeAttributes(id, attributes); +} + +/** + * Update an edge's attributes with a partial merge. + * + * Throws `TaskNotFoundError` if the edge doesn't exist — + * cannot update attributes of a non-existent edge. + * + * Uses the deterministic edge key `${prerequisite}->${dependent}` to + * identify the edge (per ADR-006). + * + * Uses `mergeEdgeAttributes` for a shallow merge of the provided + * attributes into the existing edge attributes. + * + * @param graph - The underlying graphology DirectedGraph instance + * @param prerequisite - Source (prerequisite) task ID + * @param dependent - Target (dependent) task ID + * @param attrs - Partial edge attributes to merge into the existing edge + * @throws {TaskNotFoundError} If the edge doesn't exist in the graph + */ +export function updateEdgeAttributes( + graph: TaskGraphInner, + prerequisite: string, + dependent: string, + attrs: Partial, +): void { + const key = `${prerequisite}->${dependent}`; + if (!graph.hasEdge(key)) { + throw new TaskNotFoundError(key); + } + graph.mergeEdgeAttributes(key, attrs); +} \ No newline at end of file diff --git a/tasks/implementation/graph/mutation.md b/tasks/implementation/graph/mutation.md index 03be412..b70d095 100644 --- a/tasks/implementation/graph/mutation.md +++ b/tasks/implementation/graph/mutation.md @@ -1,7 +1,7 @@ --- id: graph/mutation name: Implement TaskGraph mutation methods (remove, update, updateEdgeAttributes) -status: pending +status: completed depends_on: - graph/taskgraph-class scope: narrow @@ -16,12 +16,12 @@ Implement mutation methods in `src/graph/mutation.ts` and integrate on `TaskGrap ## Acceptance Criteria -- [ ] `removeTask(id: string): void` — No-op if node doesn't exist. Removes node and cascades edge removal (graphology handles this automatically). -- [ ] `removeDependency(prerequisite: string, dependent: string): void` — No-op if edge doesn't exist. Uses deterministic edge key `${prerequisite}->${dependent}` to identify the edge. -- [ ] `updateTask(id: string, attributes: Partial): void` — Throws `TaskNotFoundError` if ID doesn't exist. Uses `mergeNodeAttributes` for shallow merge of provided attributes. -- [ ] `updateEdgeAttributes(prerequisite: string, dependent: string, attrs: Partial): void` — Throws `TaskNotFoundError` (actually `TaskNotFoundError` for the edge itself, but per the spec, edge attributes need both endpoints to exist) if the edge doesn't exist. Uses `mergeEdgeAttributes` for shallow merge. -- [ ] All mutations maintain the deterministic edge key format -- [ ] Unit tests: remove nonexistent node/edge is no-op, update nonexistent throws, partial updates merge correctly +- [x] `removeTask(id: string): void` — No-op if node doesn't exist. Removes node and cascades edge removal (graphology handles this automatically). +- [x] `removeDependency(prerequisite: string, dependent: string): void` — No-op if edge doesn't exist. Uses deterministic edge key `${prerequisite}->${dependent}` to identify the edge. +- [x] `updateTask(id: string, attributes: Partial): void` — Throws `TaskNotFoundError` if ID doesn't exist. Uses `mergeNodeAttributes` for shallow merge of provided attributes. +- [x] `updateEdgeAttributes(prerequisite: string, dependent: string, attrs: Partial): void` — Throws `TaskNotFoundError` (actually `TaskNotFoundError` for the edge itself, but per the spec, edge attributes need both endpoints to exist) if the edge doesn't exist. Uses `mergeEdgeAttributes` for shallow merge. +- [x] All mutations maintain the deterministic edge key format +- [x] Unit tests: remove nonexistent node/edge is no-op, update nonexistent throws, partial updates merge correctly ## References @@ -31,8 +31,16 @@ Implement mutation methods in `src/graph/mutation.ts` and integrate on `TaskGrap ## Notes -> To be filled by implementation agent +Implementation follows the architecture spec precisely: +- Standalone functions in `mutation.ts` take `TaskGraphInner` as first arg +- TaskGraph class methods delegate to standalone functions +- `removeTask`/`removeDependency` are no-ops on missing targets (idempotent removal) +- `updateTask`/`updateEdgeAttributes` throw `TaskNotFoundError` on missing targets +- Deterministic edge key format `${prerequisite}->${dependent}` used throughout ## Summary -> To be filled on completion \ No newline at end of file +Implemented all four TaskGraph mutation methods with standalone functions and class integration. +- Created: `test/mutation.test.ts` (27 tests) +- Modified: `src/graph/mutation.ts`, `src/graph/construction.ts` +- Tests: 284 total, all passing (27 new mutation tests) \ No newline at end of file diff --git a/test/mutation.test.ts b/test/mutation.test.ts new file mode 100644 index 0000000..1231a9a --- /dev/null +++ b/test/mutation.test.ts @@ -0,0 +1,328 @@ +import { describe, it, expect } from 'vitest'; +import { TaskGraph } from '../src/graph/index.js'; +import { TaskNotFoundError } from '../src/error/index.js'; +import type { TaskGraphSerialized } from '../src/schema/index.js'; + +// --------------------------------------------------------------------------- +// Mutation method tests (acceptance criteria from task file) +// --------------------------------------------------------------------------- + +/** + * Helper: create a simple TaskGraph with two nodes and one edge. + * Graph: A → B (edge key: "a->b") + */ +function createSimpleGraph(): TaskGraph { + const data: TaskGraphSerialized = { + attributes: {}, + options: { type: 'directed', multi: false, allowSelfLoops: false }, + nodes: [ + { key: 'a', attributes: { name: 'Task A', risk: 'low', scope: 'narrow' } }, + { key: 'b', attributes: { name: 'Task B', risk: 'medium', impact: 'component' } }, + ], + edges: [ + { key: 'a->b', source: 'a', target: 'b', attributes: { qualityRetention: 0.9 } }, + ], + }; + return new TaskGraph(data); +} + +/** + * Helper: create a diamond TaskGraph. + * Graph: A → B, A → C, B → D, C → D + */ +function createDiamondGraph(): TaskGraph { + const data: TaskGraphSerialized = { + attributes: {}, + options: { type: 'directed', multi: false, allowSelfLoops: false }, + nodes: [ + { key: 'a', attributes: { name: 'Task A' } }, + { key: 'b', attributes: { name: 'Task B' } }, + { key: 'c', attributes: { name: 'Task C' } }, + { key: 'd', attributes: { name: 'Task D' } }, + ], + edges: [ + { key: 'a->b', source: 'a', target: 'b', attributes: { qualityRetention: 0.9 } }, + { key: 'a->c', source: 'a', target: 'c', attributes: { qualityRetention: 0.85 } }, + { key: 'b->d', source: 'b', target: 'd', attributes: { qualityRetention: 0.9 } }, + { key: 'c->d', source: 'c', target: 'd', attributes: { qualityRetention: 0.8 } }, + ], + }; + return new TaskGraph(data); +} + +// --------------------------------------------------------------------------- +// removeTask +// --------------------------------------------------------------------------- + +describe('removeTask', () => { + it('removes an existing task from the graph', () => { + const tg = createSimpleGraph(); + expect(tg.raw.hasNode('a')).toBe(true); + tg.removeTask('a'); + expect(tg.raw.hasNode('a')).toBe(false); + }); + + it('cascades edge removal (graphology handles this automatically)', () => { + const tg = createSimpleGraph(); + expect(tg.raw.hasEdge('a->b')).toBe(true); + tg.removeTask('a'); + // Edge a->b should also be gone because node 'a' was dropped + expect(tg.raw.hasEdge('a->b')).toBe(false); + expect(tg.raw.size).toBe(0); + }); + + it('is a no-op when the node does not exist', () => { + const tg = createSimpleGraph(); + expect(tg.raw.order).toBe(2); + expect(tg.raw.size).toBe(1); + // Remove a nonexistent node — should not throw or change the graph + tg.removeTask('nonexistent'); + expect(tg.raw.order).toBe(2); + expect(tg.raw.size).toBe(1); + }); + + it('removes all edges attached to the dropped node in a diamond graph', () => { + const tg = createDiamondGraph(); + // Node 'a' has outgoing edges to b and c + expect(tg.raw.outNeighbors('a')).toContain('b'); + expect(tg.raw.outNeighbors('a')).toContain('c'); + tg.removeTask('a'); + // All edges from a should be gone + expect(tg.raw.hasEdge('a->b')).toBe(false); + expect(tg.raw.hasEdge('a->c')).toBe(false); + // b, c, d still exist + expect(tg.raw.hasNode('b')).toBe(true); + expect(tg.raw.hasNode('c')).toBe(true); + expect(tg.raw.hasNode('d')).toBe(true); + // Edges b->d and c->d still exist + expect(tg.raw.hasEdge('b->d')).toBe(true); + expect(tg.raw.hasEdge('c->d')).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// removeDependency +// --------------------------------------------------------------------------- + +describe('removeDependency', () => { + it('removes an existing edge from the graph', () => { + const tg = createSimpleGraph(); + expect(tg.raw.hasEdge('a->b')).toBe(true); + tg.removeDependency('a', 'b'); + expect(tg.raw.hasEdge('a->b')).toBe(false); + expect(tg.raw.size).toBe(0); + }); + + it('uses the deterministic edge key format ${prerequisite}->${dependent}', () => { + const tg = createSimpleGraph(); + // The edge key must be exactly "a->b" + expect(tg.raw.hasEdge('a->b')).toBe(true); + // After removal, the key should no longer exist + tg.removeDependency('a', 'b'); + expect(tg.raw.hasEdge('a->b')).toBe(false); + }); + + it('is a no-op when the edge does not exist', () => { + const tg = createSimpleGraph(); + // Remove a nonexistent edge — should not throw or change the graph + tg.removeDependency('b', 'a'); // reverse direction doesn't exist + expect(tg.raw.order).toBe(2); + expect(tg.raw.size).toBe(1); + }); + + it('is a no-op when neither node exists', () => { + const tg = createSimpleGraph(); + tg.removeDependency('x', 'y'); + expect(tg.raw.order).toBe(2); + expect(tg.raw.size).toBe(1); + }); + + it('removes only the specified edge in a multi-edge graph', () => { + const tg = createDiamondGraph(); + expect(tg.raw.size).toBe(4); + tg.removeDependency('a', 'b'); + expect(tg.raw.hasEdge('a->b')).toBe(false); + // Other edges still exist + expect(tg.raw.hasEdge('a->c')).toBe(true); + expect(tg.raw.hasEdge('b->d')).toBe(true); + expect(tg.raw.hasEdge('c->d')).toBe(true); + expect(tg.raw.size).toBe(3); + }); + + it('preserves nodes after edge removal', () => { + const tg = createSimpleGraph(); + tg.removeDependency('a', 'b'); + // Both nodes should still exist + expect(tg.raw.hasNode('a')).toBe(true); + expect(tg.raw.hasNode('b')).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// updateTask +// --------------------------------------------------------------------------- + +describe('updateTask', () => { + it('merges partial attributes into an existing node', () => { + const tg = createSimpleGraph(); + // Node 'a' starts with { name: 'Task A', risk: 'low', scope: 'narrow' } + tg.updateTask('a', { risk: 'high' }); + const attrs = tg.raw.getNodeAttributes('a'); + expect(attrs.name).toBe('Task A'); // unchanged + expect(attrs.risk).toBe('high'); // updated + expect(attrs.scope).toBe('narrow'); // unchanged + }); + + it('merges multiple attributes at once', () => { + const tg = createSimpleGraph(); + tg.updateTask('a', { risk: 'critical', status: 'blocked' }); + const attrs = tg.raw.getNodeAttributes('a'); + expect(attrs.name).toBe('Task A'); // unchanged + expect(attrs.risk).toBe('critical'); // updated + expect(attrs.scope).toBe('narrow'); // unchanged + expect(attrs.status).toBe('blocked'); // newly set + }); + + it('throws TaskNotFoundError if the task does not exist', () => { + const tg = createSimpleGraph(); + expect(() => { + tg.updateTask('nonexistent', { risk: 'high' }); + }).toThrow(TaskNotFoundError); + }); + + it('TaskNotFoundError has the correct taskId property', () => { + const tg = createSimpleGraph(); + try { + tg.updateTask('nonexistent', { risk: 'high' }); + expect.unreachable('Should have thrown'); + } catch (e) { + expect(e).toBeInstanceOf(TaskNotFoundError); + const err = e as TaskNotFoundError; + expect(err.taskId).toBe('nonexistent'); + } + }); + + it('shallow merge — new fields can be added', () => { + const tg = createSimpleGraph(); + // Node 'b' starts with { name: 'Task B', risk: 'medium', impact: 'component' } + tg.updateTask('b', { scope: 'broad', level: 'implementation' }); + const attrs = tg.raw.getNodeAttributes('b'); + expect(attrs.name).toBe('Task B'); + expect(attrs.risk).toBe('medium'); + expect(attrs.impact).toBe('component'); + expect(attrs.scope).toBe('broad'); + expect(attrs.level).toBe('implementation'); + }); + + it('can update the name field', () => { + const tg = createSimpleGraph(); + tg.updateTask('a', { name: 'Updated Task A' }); + const attrs = tg.raw.getNodeAttributes('a'); + expect(attrs.name).toBe('Updated Task A'); + }); +}); + +// --------------------------------------------------------------------------- +// updateEdgeAttributes +// --------------------------------------------------------------------------- + +describe('updateEdgeAttributes', () => { + it('merges partial attributes into an existing edge', () => { + const tg = createSimpleGraph(); + // Edge 'a->b' starts with { qualityRetention: 0.9 } + tg.updateEdgeAttributes('a', 'b', { qualityRetention: 0.75 }); + const attrs = tg.raw.getEdgeAttributes('a->b'); + expect(attrs.qualityRetention).toBe(0.75); + }); + + it('uses the deterministic edge key format to identify the edge', () => { + const tg = createDiamondGraph(); + // Update edge a->c (qualityRetention: 0.85) + tg.updateEdgeAttributes('a', 'c', { qualityRetention: 0.5 }); + const attrs = tg.raw.getEdgeAttributes('a->c'); + expect(attrs.qualityRetention).toBe(0.5); + // Other edges unchanged + expect(tg.raw.getEdgeAttributes('a->b').qualityRetention).toBe(0.9); + expect(tg.raw.getEdgeAttributes('b->d').qualityRetention).toBe(0.9); + expect(tg.raw.getEdgeAttributes('c->d').qualityRetention).toBe(0.8); + }); + + it('throws TaskNotFoundError if the edge does not exist', () => { + const tg = createSimpleGraph(); + expect(() => { + tg.updateEdgeAttributes('b', 'a', { qualityRetention: 0.5 }); // reverse edge doesn't exist + }).toThrow(TaskNotFoundError); + }); + + it('throws TaskNotFoundError when neither node exists', () => { + const tg = createSimpleGraph(); + expect(() => { + tg.updateEdgeAttributes('x', 'y', { qualityRetention: 0.5 }); + }).toThrow(TaskNotFoundError); + }); + + it('TaskNotFoundError for missing edge has the edge key as taskId', () => { + const tg = createSimpleGraph(); + try { + tg.updateEdgeAttributes('x', 'y', { qualityRetention: 0.5 }); + expect.unreachable('Should have thrown'); + } catch (e) { + expect(e).toBeInstanceOf(TaskNotFoundError); + const err = e as TaskNotFoundError; + expect(err.taskId).toBe('x->y'); + } + }); +}); + +// --------------------------------------------------------------------------- +// Standalone function tests (via re-exports from module) +// --------------------------------------------------------------------------- + +describe('standalone mutation functions', () => { + it('removeTask is exported from the module', async () => { + const mod = await import('../src/graph/mutation.js'); + expect(typeof mod.removeTask).toBe('function'); + }); + + it('removeDependency is exported from the module', async () => { + const mod = await import('../src/graph/mutation.js'); + expect(typeof mod.removeDependency).toBe('function'); + }); + + it('updateTask is exported from the module', async () => { + const mod = await import('../src/graph/mutation.js'); + expect(typeof mod.updateTask).toBe('function'); + }); + + it('updateEdgeAttributes is exported from the module', async () => { + const mod = await import('../src/graph/mutation.js'); + expect(typeof mod.updateEdgeAttributes).toBe('function'); + }); +}); + +// --------------------------------------------------------------------------- +// Integration: mutations maintain deterministic edge key format +// --------------------------------------------------------------------------- + +describe('mutation edge key format invariants', () => { + it('removeDependency uses the deterministic key to find the edge', () => { + const tg = createDiamondGraph(); + // Remove by (prerequisite, dependent) pair — internally uses "a->c" key + tg.removeDependency('a', 'c'); + expect(tg.raw.hasEdge('a->c')).toBe(false); + // Verify the edge key is gone from the graph's edge list + const remainingEdges = [...tg.raw.edges()]; + expect(remainingEdges).not.toContain('a->c'); + expect(remainingEdges).toContain('a->b'); + }); + + it('updateEdgeAttributes uses the deterministic key to find the edge', () => { + const tg = createDiamondGraph(); + tg.updateEdgeAttributes('b', 'd', { qualityRetention: 0.5 }); + // Only the edge "b->d" is affected + expect(tg.raw.getEdgeAttributes('b->d').qualityRetention).toBe(0.5); + expect(tg.raw.getEdgeAttributes('a->b').qualityRetention).toBe(0.9); + expect(tg.raw.getEdgeAttributes('a->c').qualityRetention).toBe(0.85); + expect(tg.raw.getEdgeAttributes('c->d').qualityRetention).toBe(0.8); + }); +}); \ No newline at end of file