feat(cost-benefit/workflow-cost): fix includeCompleted default to false per api-surface.md spec
The workflowCost function had includeCompleted defaulting to true, but the api-surface.md specifies the default should be false. Fixed the default and updated test suite to verify the correct default behavior and add explicit test for includeCompleted: true opt-in case. All 562 tests passing across 12 test files.
This commit is contained in:
@@ -160,7 +160,7 @@ export function workflowCost(
|
|||||||
): WorkflowCostResult {
|
): WorkflowCostResult {
|
||||||
const propagationMode = options?.propagationMode ?? "dag-propagate";
|
const propagationMode = options?.propagationMode ?? "dag-propagate";
|
||||||
const defaultQualityRetention = options?.defaultQualityRetention ?? 0.9;
|
const defaultQualityRetention = options?.defaultQualityRetention ?? 0.9;
|
||||||
const includeCompleted = options?.includeCompleted ?? true;
|
const includeCompleted = options?.includeCompleted ?? false;
|
||||||
|
|
||||||
// Get topological order — throws CircularDependencyError if cyclic
|
// Get topological order — throws CircularDependencyError if cyclic
|
||||||
const topoOrder = topologicalOrder(graph);
|
const topoOrder = topologicalOrder(graph);
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
---
|
---
|
||||||
id: cost-benefit/workflow-cost
|
id: cost-benefit/workflow-cost
|
||||||
name: Implement workflowCost orchestration function
|
name: Implement workflowCost orchestration function
|
||||||
status: pending
|
status: completed
|
||||||
depends_on:
|
depends_on:
|
||||||
- cost-benefit/dag-propagation
|
- cost-benefit/dag-propagation
|
||||||
- graph/queries
|
- graph/queries
|
||||||
@@ -17,18 +17,18 @@ Implement `workflowCost(graph: TaskGraph, options?: WorkflowCostOptions): Workfl
|
|||||||
|
|
||||||
## Acceptance Criteria
|
## Acceptance Criteria
|
||||||
|
|
||||||
- [ ] `workflowCost` accepts `WorkflowCostOptions` with optional: `includeCompleted`, `limit`, `propagationMode`, `defaultQualityRetention`
|
- [x] `workflowCost` accepts `WorkflowCostOptions` with optional: `includeCompleted`, `limit`, `propagationMode`, `defaultQualityRetention`
|
||||||
- [ ] Default propagation mode: `"dag-propagate"` per ADR-004
|
- [x] Default propagation mode: `"dag-propagate"` per ADR-004
|
||||||
- [ ] Default `defaultQualityRetention`: 0.9
|
- [x] Default `defaultQualityRetention`: 0.9
|
||||||
- [ ] Each task in result includes: `taskId`, `name`, `ev`, `pIntrinsic`, `pEffective`, `probability` (= `pEffective`), `scopeCost`, `impactWeight`
|
- [x] 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`)
|
- [x] `totalEv`: sum of all task EVs (excluding completed tasks from output when `includeCompleted: false`)
|
||||||
- [ ] `averageEv`: `totalEv / tasks.length`
|
- [x] `averageEv`: `totalEv / tasks.length`
|
||||||
- [ ] `propagationMode`: reflected in result
|
- [x] `propagationMode`: reflected in result
|
||||||
- [ ] When `includeCompleted: false`: completed tasks excluded from `tasks` array but remain in propagation chain with p=1.0
|
- [x] 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
|
- [x] 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)
|
- [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)
|
||||||
- [ ] Throws `CircularDependencyError` if graph is cyclic
|
- [x] Throws `CircularDependencyError` if graph is cyclic
|
||||||
- [ ] Unit tests: full workflow cost calculation, independent vs dag-propagate comparison, excludeCompleted scenarios, limit behavior
|
- [x] Unit tests: full workflow cost calculation, independent vs dag-propagate comparison, excludeCompleted scenarios, limit behavior
|
||||||
|
|
||||||
## References
|
## References
|
||||||
|
|
||||||
@@ -38,8 +38,11 @@ Implement `workflowCost(graph: TaskGraph, options?: WorkflowCostOptions): Workfl
|
|||||||
|
|
||||||
## Notes
|
## 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
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
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
|
||||||
@@ -774,15 +774,33 @@ describe("workflowCost", () => {
|
|||||||
expect(taskB.pIntrinsic).toBeCloseTo(0.80);
|
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([
|
const graph = TaskGraph.fromTasks([
|
||||||
{ id: "A", name: "Task A", dependsOn: [], risk: "high", scope: "narrow", impact: "isolated", status: "completed" },
|
{ 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" },
|
{ 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")!;
|
const taskA = result.tasks.find(t => t.taskId === "A")!;
|
||||||
expect(taskA).toBeDefined();
|
expect(taskA).toBeDefined();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user