diff --git a/src/analysis/cost-benefit.ts b/src/analysis/cost-benefit.ts index 43cd50c..717ddfb 100644 --- a/src/analysis/cost-benefit.ts +++ b/src/analysis/cost-benefit.ts @@ -160,7 +160,7 @@ export function workflowCost( ): WorkflowCostResult { const propagationMode = options?.propagationMode ?? "dag-propagate"; const defaultQualityRetention = options?.defaultQualityRetention ?? 0.9; - const includeCompleted = options?.includeCompleted ?? true; + const includeCompleted = options?.includeCompleted ?? false; // Get topological order — throws CircularDependencyError if cyclic const topoOrder = topologicalOrder(graph); diff --git a/tasks/implementation/cost-benefit/workflow-cost.md b/tasks/implementation/cost-benefit/workflow-cost.md index 8228760..531b376 100644 --- a/tasks/implementation/cost-benefit/workflow-cost.md +++ b/tasks/implementation/cost-benefit/workflow-cost.md @@ -1,7 +1,7 @@ --- id: cost-benefit/workflow-cost name: Implement workflowCost orchestration function -status: pending +status: completed depends_on: - cost-benefit/dag-propagation - graph/queries @@ -17,18 +17,18 @@ Implement `workflowCost(graph: TaskGraph, options?: WorkflowCostOptions): Workfl ## Acceptance Criteria -- [ ] `workflowCost` accepts `WorkflowCostOptions` with optional: `includeCompleted`, `limit`, `propagationMode`, `defaultQualityRetention` -- [ ] Default propagation mode: `"dag-propagate"` per ADR-004 -- [ ] Default `defaultQualityRetention`: 0.9 -- [ ] Each task in result includes: `taskId`, `name`, `ev`, `pIntrinsic`, `pEffective`, `probability` (= `pEffective`), `scopeCost`, `impactWeight` -- [ ] `totalEv`: sum of all task EVs (excluding completed tasks from output when `includeCompleted: false`) -- [ ] `averageEv`: `totalEv / tasks.length` -- [ ] `propagationMode`: reflected in result -- [ ] When `includeCompleted: false`: completed tasks excluded from `tasks` array but remain in propagation chain with p=1.0 -- [ ] When `includeCompleted: false`: only `"completed"` status triggers exclusion; `"failed"` and `"blocked"` are always included -- [ ] When `limit` is set: returns at most `limit` tasks (sorted by EV descending? or topological order? spec says "limits the number of tasks in the result" — use topological order with limit) -- [ ] Throws `CircularDependencyError` if graph is cyclic -- [ ] Unit tests: full workflow cost calculation, independent vs dag-propagate comparison, excludeCompleted scenarios, limit behavior +- [x] `workflowCost` accepts `WorkflowCostOptions` with optional: `includeCompleted`, `limit`, `propagationMode`, `defaultQualityRetention` +- [x] Default propagation mode: `"dag-propagate"` per ADR-004 +- [x] Default `defaultQualityRetention`: 0.9 +- [x] Each task in result includes: `taskId`, `name`, `ev`, `pIntrinsic`, `pEffective`, `probability` (= `pEffective`), `scopeCost`, `impactWeight` +- [x] `totalEv`: sum of all task EVs (excluding completed tasks from output when `includeCompleted: false`) +- [x] `averageEv`: `totalEv / tasks.length` +- [x] `propagationMode`: reflected in result +- [x] When `includeCompleted: false`: completed tasks excluded from `tasks` array but remain in propagation chain with p=1.0 +- [x] When `includeCompleted: false`: only `"completed"` status triggers exclusion; `"failed"` and `"blocked"` are always included +- [x] When `limit` is set: returns at most `limit` tasks (sorted by EV descending? or topological order? spec says "limits the number of tasks in the result" — use topological order with limit) +- [x] Throws `CircularDependencyError` if graph is cyclic +- [x] Unit tests: full workflow cost calculation, independent vs dag-propagate comparison, excludeCompleted scenarios, limit behavior ## References @@ -38,8 +38,11 @@ Implement `workflowCost(graph: TaskGraph, options?: WorkflowCostOptions): Workfl ## Notes -> To be filled by implementation agent +Fixed `includeCompleted` default from `true` to `false` to match the api-surface.md specification. The implementation was mostly complete from prior dependency tasks (dag-propagation), but the default was incorrect. Updated the test suite to verify the correct default behavior and explicitly test the `includeCompleted: true` opt-in case. ## Summary -> To be filled on completion \ No newline at end of file +Implemented `workflowCost` orchestration function and fixed `includeCompleted` default to `false` per api-surface.md. +- Modified: `src/analysis/cost-benefit.ts` — fixed `includeCompleted` default from `true` to `false` +- Modified: `test/cost-benefit.test.ts` — updated default-behavior test and added explicit `includeCompleted: true` test +- Tests: 64 cost-benefit tests, all passing; 562 total tests across 12 test files, all passing \ No newline at end of file diff --git a/test/cost-benefit.test.ts b/test/cost-benefit.test.ts index 49ba977..65de72e 100644 --- a/test/cost-benefit.test.ts +++ b/test/cost-benefit.test.ts @@ -774,15 +774,33 @@ describe("workflowCost", () => { expect(taskB.pIntrinsic).toBeCloseTo(0.80); }); - it("includes completed tasks when includeCompleted=true (default)", () => { + it("excludes completed tasks by default (includeCompleted defaults to false)", () => { const graph = TaskGraph.fromTasks([ { id: "A", name: "Task A", dependsOn: [], risk: "high", scope: "narrow", impact: "isolated", status: "completed" }, { id: "B", name: "Task B", dependsOn: ["A"], risk: "medium", scope: "narrow", impact: "isolated" }, ]); - const result = workflowCost(graph.raw); // default includeCompleted=true + const result = workflowCost(graph.raw); // default includeCompleted=false - // A should appear in the task list + // A should NOT appear in the task list (default behavior) + expect(result.tasks.find(t => t.taskId === "A")).toBeUndefined(); + + // B's propagation uses p=1.0 for A (completed) + // inheritedFromA = 1.0 + (1-1.0)*0.9 = 1.0 + // pEffective_B = 0.80 * 1.0 = 0.80 + const taskB = result.tasks.find(t => t.taskId === "B")!; + expect(taskB.pEffective).toBeCloseTo(0.80); + }); + + it("includes completed tasks when includeCompleted=true", () => { + const graph = TaskGraph.fromTasks([ + { id: "A", name: "Task A", dependsOn: [], risk: "high", scope: "narrow", impact: "isolated", status: "completed" }, + { id: "B", name: "Task B", dependsOn: ["A"], risk: "medium", scope: "narrow", impact: "isolated" }, + ]); + + const result = workflowCost(graph.raw, { includeCompleted: true }); + + // A should appear in the task list when explicitly included const taskA = result.tasks.find(t => t.taskId === "A")!; expect(taskA).toBeDefined();