From 2d4dbf758faa3a2910a4bcd5ad920977f5f5df2d Mon Sep 17 00:00:00 2001 From: reference-week Date: Sat, 5 Sep 2026 12:45:56 +0200 Subject: [PATCH 1/2] fix(cli): an invoked step does not close the orchestration that invoked it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading every `step_start` as the end of whatever was open assumes a session only ever runs one skill after another. An orchestrating skill's whole job is to invoke others, so that assumption cuts it short at its first child. Measured on the one orchestrated session captured, 2026-09-04: `aidd-orchestrator:01-sdlc` opened at 05:56:27 and `aidd-pm:04-spec` at 05:59:53, so the orchestration read as 206 seconds against a session that ran until 09:27:21. `by_flow`, reading the same journal under the rule this adopts, named 1,052 records for that skill while `by_step` named 1. `buildStepIntervals` is now two walks over the same lines. The orchestrating half *is* `buildFlowIntervals` — a flow is an orchestrating step, and calling it rather than restating it is what keeps the two axes from drifting apart again. The other half opens on a non-orchestrating `step_start` and keeps the rule it always had: any later `step_start` closes it, because a session that starts orchestrating is no longer running the plain skill it was running before. `attributeMoment` now answers with the innermost interval around a moment rather than the first one in the array. Both claims are true of a record inside an invoked step; the inner one says more, and the outer one goes on answering for every moment the inner one does not cover. Array order decides nothing, which matters now that two separate walks build the list. Which skills orchestrate is `ORCHESTRATING_SKILLS`'s declaration, never inferred: nesting and sequence produce the identical journal, so no rule read off the boundaries alone separates them. The limit is stated in code — a skill that invokes another without being declared an orchestrator is still cut short by its own child. **Measured effect on today's data: none.** `by_step` reads `aidd-dev:01-plan=972, aidd-orchestrator:01-sdlc=1` before and after. The tail of that session is owned by `aidd-dev:01-plan`, an invoked step nothing closed, which runs to the journal's last witnessed moment and wins on having started later. Telling an interval closed by evidence from one merely bounded is the next change, and this two-walk shape is what it needs to fall back to. Mutations run, both killed: the outermost interval winning, and a single walk opening on every `step_start`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VWNxk63AGKkqE8HRqHLjGp --- cli/src/domain/models/step-attribution.ts | 80 +++++++++++++++++-- .../models/step-attribution.unit.test.ts | 73 +++++++++++++++++ 2 files changed, 146 insertions(+), 7 deletions(-) diff --git a/cli/src/domain/models/step-attribution.ts b/cli/src/domain/models/step-attribution.ts index 5b632ce2..06a982ad 100644 --- a/cli/src/domain/models/step-attribution.ts +++ b/cli/src/domain/models/step-attribution.ts @@ -5,6 +5,7 @@ import type { RunJournalStepStart, RunJournalTaskDeclared, } from "../ports/run-journal-reader.js"; +import { buildFlowIntervals, ORCHESTRATING_SKILLS } from "./flow-attribution.js"; import { buildClosedIntervals, type ClosedInterval } from "./journal-intervals.js"; import { namesTheSameSkill } from "./skill-name.js"; @@ -93,9 +94,13 @@ export interface StepInterval extends ClosedInterval { * Two runs of the very same skill in one session yield two distinct intervals, never one * merged by name, exactly as the boundaries dictate; nothing here decides which record * falls into which, that is `attributeMoment`'s job. */ -export function buildStepIntervals( +/** Every step a session opened that does not orchestrate - each closed by its own + * `step_end`, by the next `step_start` whatever that one is, or by the journal's own last + * witnessed moment. Two ordinary skills in a row are a sequence, so the second ends the + * first; that reading is unchanged. */ +function buildInvokedStepIntervals( journal: RunJournal, - periodEndMs?: number + periodEndMs: number | undefined ): readonly StepInterval[] { return buildClosedIntervals< RunJournalBoundary | RunJournalTaskDeclared | RunJournalFileWritten, @@ -104,17 +109,80 @@ export function buildStepIntervals( >( [...journal.boundaries, ...journal.taskDeclarations, ...journal.filesWritten], periodEndMs, - (boundary): boundary is RunJournalStepStart => boundary.type === "step_start", + (boundary): boundary is RunJournalStepStart => + boundary.type === "step_start" && !ORCHESTRATING_SKILLS.has(boundary.skill), + // Any `step_start` closes one of these, an orchestrating one included: a session that + // starts orchestrating is no longer running the plain skill it was running before. + // `isOpener` already covers the non-orchestrating half; naming the whole rule here is + // what keeps the orchestrating half from being an omission nobody wrote down. (boundary, opener) => - boundary.type === "step_end" && namesTheSameSkill(boundary.skill, opener.skill), + boundary.type === "step_start" || + (boundary.type === "step_end" && namesTheSameSkill(boundary.skill, opener.skill)), (opener, startMs, endMs) => ({ skill: opener.skill, startMs, endMs }) ); } +/** + * Journal lines in, closed intervals out - no filesystem, no record. + * + * **An invoked step no longer closes the orchestration that invoked it**, changed + * 2026-09-05. Reading every `step_start` as the end of whatever was open assumes a session + * only ever runs one skill after another, and an orchestrating skill's whole job is to + * invoke others. Measured on the one orchestrated session captured, 2026-09-04: + * `aidd-orchestrator:01-sdlc` opened at 05:56:27 and `aidd-pm:04-spec` at 05:59:53, so the + * orchestration read as 206 seconds against a session that ran until 09:27:21 - which is + * why this axis named 1 record for that skill while `by_flow`, reading the same journal + * under the rule this now adopts, named 1,052. + * + * Which skills orchestrate is `ORCHESTRATING_SKILLS`'s declaration, never inferred from the + * lines: nesting and sequence produce the identical journal, so no rule read off the + * boundaries alone can separate them. That is also the limit - a skill that invokes another + * without being declared an orchestrator is still read as a sequence, and is still cut short + * by its own child. + * + * Built as two walks over the same lines rather than one with a branch inside it. The + * orchestrating half **is** `buildFlowIntervals` - a flow is an orchestrating step, and + * saying so by calling it is what keeps the two axes from drifting apart again. + */ +export function buildStepIntervals( + journal: RunJournal, + periodEndMs?: number +): readonly StepInterval[] { + return [ + ...buildFlowIntervals(journal, periodEndMs), + ...buildInvokedStepIntervals(journal, periodEndMs), + ]; +} + /** Where a record's own moment falls inside one interval, that interval's skill is the * attribution, marked as derived. A record with no moment, or one earlier than every * interval, is unattributed — never folded into the first step, which would assume work * began the instant a marker happened to be written rather than sometime before it. */ +/** The most specific interval a moment falls in: the latest to have opened, and among + * equals the first to close. An invoked step and the orchestration around it both contain + * the moment, and both claims are true - the inner one is the one that says more, and the + * outer one goes on answering for every moment the inner one does not cover. Order in the + * array decides nothing: the two walks that build these run separately, so a rule that + * read the first match would answer differently for the same journal depending on which + * walk happened to run first. */ +function innermostAround( + intervals: readonly StepInterval[], + momentMs: number +): StepInterval | undefined { + let best: StepInterval | undefined; + for (const interval of intervals) { + if (momentMs < interval.startMs || momentMs >= interval.endMs) continue; + if ( + best === undefined || + interval.startMs > best.startMs || + (interval.startMs === best.startMs && interval.endMs < best.endMs) + ) { + best = interval; + } + } + return best; +} + export function attributeMoment( intervals: readonly StepInterval[], momentIso: string | undefined @@ -122,8 +190,6 @@ export function attributeMoment( if (momentIso === undefined) return UNATTRIBUTED; const momentMs = Date.parse(momentIso); if (Number.isNaN(momentMs)) return UNATTRIBUTED; - const hit = intervals.find( - (interval) => momentMs >= interval.startMs && momentMs < interval.endMs - ); + const hit = innermostAround(intervals, momentMs); return hit ? { source: "journal-interval", step: hit.skill } : UNATTRIBUTED; } diff --git a/cli/tests/domain/models/step-attribution.unit.test.ts b/cli/tests/domain/models/step-attribution.unit.test.ts index c8059be7..24480af4 100644 --- a/cli/tests/domain/models/step-attribution.unit.test.ts +++ b/cli/tests/domain/models/step-attribution.unit.test.ts @@ -256,6 +256,79 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = expect(attributeMoment(intervals, A_START.at)).toEqual({ source: "unattributed" }); }); + // An orchestrating skill invokes others; that is what `ORCHESTRATING_SKILLS` declares it + // does. Reading the invoked skill's own `step_start` as the end of the orchestration + // credits an orchestration that ran for hours with the seconds before its first child. + // Measured on the one orchestrated session captured, 2026-09-04: `aidd-orchestrator:01-sdlc` + // opened at 05:56:27 and `aidd-pm:04-spec` opened at 05:59:53, so the orchestration was + // read as 206 seconds long against a session that ran until 09:27:21. The flow axis, which + // already refuses to let a non-orchestrating start close one, named 1,052 records for that + // same skill while this axis named 1. + it("does not let an invoked step close the orchestration that invoked it", () => { + const intervals = buildStepIntervals( + journalOf( + { type: "step_start", at: "2026-08-20T10:00:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "step_start", at: "2026-08-20T10:05:00Z", skill: "aidd-pm:04-spec" }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" } + ) + ); + + const sdlc = intervals.find((interval) => interval.skill === "aidd-orchestrator:01-sdlc"); + expect(sdlc?.endMs).toBe(Date.parse("2026-08-20T11:00:00Z")); + }); + + // The invoked step is inside the orchestration, not beside it, so both intervals contain + // the same moment. The innermost is the one that answers: it is the more specific claim, + // and the outer one is still true of it. + it("attributes a moment inside both to the step, and one outside it to the orchestration", () => { + const intervals = buildStepIntervals( + journalOf( + { type: "step_start", at: "2026-08-20T10:00:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "step_start", at: "2026-08-20T10:05:00Z", skill: "aidd-pm:04-spec" }, + { type: "step_end", at: "2026-08-20T10:10:00Z", skill: "aidd-pm:04-spec" }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" } + ) + ); + + expect(attributeMoment(intervals, "2026-08-20T10:02:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-orchestrator:01-sdlc", + }); + expect(attributeMoment(intervals, "2026-08-20T10:07:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-pm:04-spec", + }); + // Past the invoked step's own declared end, back inside the orchestration alone. + expect(attributeMoment(intervals, "2026-08-20T10:30:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-orchestrator:01-sdlc", + }); + }); + + // Nesting is declared, never inferred: only a skill `ORCHESTRATING_SKILLS` names invokes + // others. Two ordinary skills in a row are a sequence, and the second still ends the first. + it("still lets one ordinary step close another, which is a sequence and not a nesting", () => { + const intervals = buildStepIntervals(journalOf(A_START, B_START, TURN_END)); + + const first = intervals.find((interval) => interval.skill === A_START.skill); + expect(first?.endMs).toBe(Date.parse(B_START.at)); + }); + + // One orchestration does not nest inside another by default - the same rule + // `buildFlowIntervals` already applies to the wider concept, read from the same lines. + it("lets one orchestration close another", () => { + const intervals = buildStepIntervals( + journalOf( + { type: "step_start", at: "2026-08-20T10:00:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "step_start", at: "2026-08-20T10:05:00Z", skill: "aidd-orchestrator:02-backlog" }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" } + ) + ); + + const first = intervals.find((interval) => interval.skill === "aidd-orchestrator:01-sdlc"); + expect(first?.endMs).toBe(Date.parse("2026-08-20T10:05:00Z")); + }); + it("touches no filesystem — the module imports none of Node's fs APIs", () => { const url = new URL("../../../src/domain/models/step-attribution.ts", import.meta.url); const source = readFileSync(fileURLToPath(url), "utf8"); From f9cc34c4c4fb9788979cef0b80d9c2ea00826fa5 Mon Sep 17 00:00:00 2001 From: reference-week Date: Sat, 5 Sep 2026 13:01:51 +0200 Subject: [PATCH 2/2] fix(cli): a step nothing closed yields to the one that encloses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An interval nothing ever closed ends at the journal's own last witnessed moment. That end is a bound, not a measurement, and reading it as an extent is how a step that opened shortly before a long session went on working comes to be credited with all of it — purely for having opened later than the orchestration around it. Measured on the one orchestrated session captured, 2026-09-04: `aidd-dev:01-plan` opened at 06:00:50 inside an orchestration opened at 05:56:27. Neither was ever closed, so both ran to 09:27:21 and the innermost-start rule gave the invoked step every record in between. `buildClosedIntervals` now tells its caller which of the two ended an interval — a moment the journal witnessed, or the cap standing in for one it never did — and `answersFor` reads it: an interval closed by the journal's end yields to one that encloses it and was never closed either. Two values and not three. "Closed by its own `step_end`" and "closed by a later `step_start`" are different strengths of evidence and both were weighed; both are nevertheless a moment the journal witnessed, and no caller separates them, so a third value would be structure nothing reads. The yielding is that narrow and no wider. Where the enclosing interval states its own end, the inner one runs past it, nothing encloses it, and the innermost claim stands — the same answer it gets when both ends are witnessed. An unclosed interval nothing encloses still answers for its own moments: what is refused is preferring a bound over a wider claim covering the same moment, never the bound. No tie between two unclosed sibling steps can arise, and it is not this rule that prevents it: any `step_start` closes whichever plain step was open, so at most one invoked step is ever left unclosed at a time. The test says so by demonstration rather than a comment. Containment between two unclosed intervals is compared on the start alone. They all end at the same cap, so comparing the ends would be a clause no input can make false — the kind of default that hides a wrong number. Measured on the real sink, 30 days, 31,025 requests, every axis reconciling: by_step aidd-orchestrator:01-sdlc journal-interval 1 -> 973 by_step aidd-dev:01-plan journal-interval 972 -> 0 973 is the whole of what the two walks can agree on. 1,052 records fall inside that flow interval; 79 of them carry a step their own tool stated (12 `aidd-orchestrator:01-sdlc`, 61 `aidd-dev:01-plan`, 6 `aidd-pm:04-spec`) and keep that stronger claim on the step axis. 1,052 - 79 = 973, and `by_flow` reads 1,052 for the same interval. This moves attribution within the 1,052 records the journal covers. It does not move the 27,393 unattributed ones: those predate the journal, from one session resumed since 2026-08-13, and no interval rule reaches them. Mutations run, three killed by three distinct guards: never yielding, yielding to any earlier start, and letting a closed interval yield too. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VWNxk63AGKkqE8HRqHLjGp --- cli/src/domain/models/flow-attribution.ts | 12 +++- cli/src/domain/models/journal-intervals.ts | 34 +++++++-- cli/src/domain/models/step-attribution.ts | 67 ++++++++++++++--- .../display/cost-report-artefact.unit.test.ts | 1 + .../cost-report-order.property.unit.test.ts | 1 + .../domain/models/cost-report.unit.test.ts | 10 +++ .../models/flow-attribution.unit.test.ts | 15 +++- .../models/step-attribution.unit.test.ts | 71 +++++++++++++++++++ 8 files changed, 195 insertions(+), 16 deletions(-) diff --git a/cli/src/domain/models/flow-attribution.ts b/cli/src/domain/models/flow-attribution.ts index 035a9641..833fe708 100644 --- a/cli/src/domain/models/flow-attribution.ts +++ b/cli/src/domain/models/flow-attribution.ts @@ -5,7 +5,11 @@ import type { RunJournalStepStart, RunJournalTaskDeclared, } from "../ports/run-journal-reader.js"; -import { buildClosedIntervals, type ClosedInterval } from "./journal-intervals.js"; +import { + buildClosedIntervals, + type ClosedInterval, + type IntervalClosure, +} from "./journal-intervals.js"; import { namesTheSameSkill } from "./skill-name.js"; /** @@ -104,6 +108,10 @@ export function bareOrchestratingSkillNames( * none at all. */ export interface FlowInterval extends ClosedInterval { readonly skill: string; + /** Whether `endMs` is a moment this journal witnessed or the cap standing in for one it + * never did. Carried because `buildStepIntervals` composes these into the step axis and + * reads it there; no flow row of its own is printed differently for it. */ + readonly closedBy: IntervalClosure; } /** @@ -147,6 +155,6 @@ export function buildFlowIntervals( boundary.type === "step_start" && ORCHESTRATING_SKILLS.has(boundary.skill), (boundary, opener) => boundary.type === "step_end" && namesTheSameSkill(boundary.skill, opener.skill), - (opener, startMs, endMs) => ({ skill: opener.skill, startMs, endMs }) + (opener, startMs, endMs, closedBy) => ({ skill: opener.skill, startMs, endMs, closedBy }) ); } diff --git a/cli/src/domain/models/journal-intervals.ts b/cli/src/domain/models/journal-intervals.ts index ed146ff4..e7af8c06 100644 --- a/cli/src/domain/models/journal-intervals.ts +++ b/cli/src/domain/models/journal-intervals.ts @@ -41,6 +41,22 @@ function cappedLastMoment(witnessedLastMs: number, periodEndMs: number | undefin return periodEndMs === undefined ? witnessedLastMs : Math.min(witnessedLastMs, periodEndMs); } +/** What ended an interval. `boundary` is a moment the journal actually witnessed - a + * closer, or a later opener; `journal-end` is the fallback, meaning nothing in the journal + * ever said this interval was over and it was capped at the last moment the journal + * witnessed at all. + * + * The distinction is a caller's to act on, not this walk's: an interval capped at the + * journal's end is a bound, and reading it as a measured extent is how a step that opened + * shortly before a long session went quiet comes to be credited with everything that + * followed. `step-attribution.ts` is the caller that reads it today. + * + * Two values and not three. "Closed by its own `step_end`" and "closed by a later opener" + * are genuinely different strengths of evidence, and both were considered; both are + * nevertheless a moment the journal witnessed, and no caller distinguishes them, so a third + * value would be structure nothing reads. */ +export type IntervalClosure = "boundary" | "journal-end"; + /** The two facts every closed interval this module builds actually needs — `path` * (`TaskInterval`) or `skill` (`FlowInterval`) rides beside these, never inside this shape * itself. */ @@ -81,8 +97,8 @@ export function momentFallsWithin( * interval would go on attributing everything a long-running session does afterward to the * first opener it ever saw. * - * `toInterval` turns one opener plus its resolved bounds into the caller's own interval - * shape, or `null` to close the interval without emitting a row for it - + * `toInterval` turns one opener plus its resolved bounds, and how those bounds were + * reached (`IntervalClosure`), into the caller's own interval shape, or `null` to close the interval without emitting a row for it - * `buildTaskIntervals` uses this to skip a declared path `taskIdentityFromWrittenPath` * cannot resolve while still letting it close whatever interval came before it; * `buildFlowIntervals` never returns `null`, since every orchestrating `step_start` names a @@ -97,7 +113,12 @@ export function buildClosedIntervals< periodEndMs: number | undefined, isOpener: (boundary: TBoundary) => boundary is TOpener, isCloser: (boundary: TBoundary, opener: TOpener) => boolean, - toInterval: (opener: TOpener, startMs: number, endMs: number) => TInterval | null + toInterval: ( + opener: TOpener, + startMs: number, + endMs: number, + closedBy: IntervalClosure + ) => TInterval | null ): readonly TInterval[] { const everyWitnessedMoment = timed(boundaryLike); // Not one readable moment in the whole journal: no interval either, and nothing below @@ -112,8 +133,11 @@ export function buildClosedIntervals< for (let i = 0; i < everyWitnessedMoment.length; i++) { const { atMs: startMs, boundary } = everyWitnessedMoment[i]; if (!isOpener(boundary)) continue; - const endMs = firstCloserAfter(everyWitnessedMoment, i, boundary, isOpener, isCloser) ?? lastMs; - const interval = toInterval(boundary, startMs, endMs); + const closerMs = firstCloserAfter(everyWitnessedMoment, i, boundary, isOpener, isCloser); + const interval = + closerMs === undefined + ? toInterval(boundary, startMs, lastMs, "journal-end") + : toInterval(boundary, startMs, closerMs, "boundary"); if (interval !== null) intervals.push(interval); } return intervals; diff --git a/cli/src/domain/models/step-attribution.ts b/cli/src/domain/models/step-attribution.ts index 06a982ad..5877db6b 100644 --- a/cli/src/domain/models/step-attribution.ts +++ b/cli/src/domain/models/step-attribution.ts @@ -6,7 +6,11 @@ import type { RunJournalTaskDeclared, } from "../ports/run-journal-reader.js"; import { buildFlowIntervals, ORCHESTRATING_SKILLS } from "./flow-attribution.js"; -import { buildClosedIntervals, type ClosedInterval } from "./journal-intervals.js"; +import { + buildClosedIntervals, + type ClosedInterval, + type IntervalClosure, +} from "./journal-intervals.js"; import { namesTheSameSkill } from "./skill-name.js"; /** How a record's step came to be known. Never collapsed into one field with the step @@ -75,6 +79,10 @@ const UNATTRIBUTED: StepAttribution = { source: "unattributed" }; * Claude Code, Cursor and OpenCode by `journal.cjs`'s own `HOOK_EVENT_NAME_TO_CANONICAL`. */ export interface StepInterval extends ClosedInterval { readonly skill: string; + /** Whether `endMs` is a moment the journal witnessed or the cap standing in for one it + * never did - `answersFor` reads it, and it is the whole reason the cap above is safe to + * apply. */ + readonly closedBy: IntervalClosure; } /** Journal lines in, closed intervals out - no filesystem, no record. Run through the one @@ -118,7 +126,7 @@ function buildInvokedStepIntervals( (boundary, opener) => boundary.type === "step_start" || (boundary.type === "step_end" && namesTheSameSkill(boundary.skill, opener.skill)), - (opener, startMs, endMs) => ({ skill: opener.skill, startMs, endMs }) + (opener, startMs, endMs, closedBy) => ({ skill: opener.skill, startMs, endMs, closedBy }) ); } @@ -165,13 +173,9 @@ export function buildStepIntervals( * array decides nothing: the two walks that build these run separately, so a rule that * read the first match would answer differently for the same journal depending on which * walk happened to run first. */ -function innermostAround( - intervals: readonly StepInterval[], - momentMs: number -): StepInterval | undefined { +function innermostOf(intervals: readonly StepInterval[]): StepInterval | undefined { let best: StepInterval | undefined; for (const interval of intervals) { - if (momentMs < interval.startMs || momentMs >= interval.endMs) continue; if ( best === undefined || interval.startMs > best.startMs || @@ -183,6 +187,53 @@ function innermostAround( return best; } +/** Whether an interval nothing closed sits inside another that nothing closed either. + * + * Every unclosed interval ends at the same moment - the journal's own last witnessed one, + * capped identically for all of them - so containment between two of them reduces to which + * opened first, and comparing the ends would be a clause no input can make false. The + * enclosing one is the answer because the inner one's extent rests on no evidence at all, + * while the enclosing one is at least still known to have been open at that moment. */ +function enclosedByAnotherUnclosed( + covering: readonly StepInterval[], + interval: StepInterval +): boolean { + if (interval.closedBy !== "journal-end") return false; + return covering.some( + (other) => other.closedBy === "journal-end" && other.startMs < interval.startMs + ); +} + +/** The interval that answers for a moment. + * + * The innermost one covering it, *except* that an interval nothing ever closed yields to + * one that encloses it and was never closed either. An unclosed interval ends at the + * journal's own last witnessed moment, so its extent is a bound and not a measurement; a + * step opened shortly before a long session goes on working would otherwise be credited + * with all of it, purely for having opened later than the orchestration around it. + * Measured on the one orchestrated session captured, 2026-09-04: 972 records attributed to + * `aidd-dev:01-plan`, opened at 06:00:50 and never closed, inside an orchestration opened + * at 05:56:27 and never closed either. + * + * Yielding is between two unclosed intervals and no wider. Where the enclosing interval + * states its own end, the inner one runs past it and nothing encloses it, so the innermost + * claim stands - the same answer it gets when both ends are witnessed. And an unclosed + * interval that nothing encloses still answers: what is refused is preferring a bound over + * a wider claim that covers the same moment, never the bound itself. + * + * No tie between two unclosed *sibling* steps can arise to be broken here, and it is not + * this function that prevents it: any `step_start` closes whichever plain step was open, so + * at most one invoked step is ever left unclosed at a time. */ +function answersFor( + intervals: readonly StepInterval[], + momentMs: number +): StepInterval | undefined { + const covering = intervals.filter( + (interval) => momentMs >= interval.startMs && momentMs < interval.endMs + ); + return innermostOf(covering.filter((interval) => !enclosedByAnotherUnclosed(covering, interval))); +} + export function attributeMoment( intervals: readonly StepInterval[], momentIso: string | undefined @@ -190,6 +241,6 @@ export function attributeMoment( if (momentIso === undefined) return UNATTRIBUTED; const momentMs = Date.parse(momentIso); if (Number.isNaN(momentMs)) return UNATTRIBUTED; - const hit = innermostAround(intervals, momentMs); + const hit = answersFor(intervals, momentMs); return hit ? { source: "journal-interval", step: hit.skill } : UNATTRIBUTED; } diff --git a/cli/tests/application/display/cost-report-artefact.unit.test.ts b/cli/tests/application/display/cost-report-artefact.unit.test.ts index 7583f06f..25f0ecbb 100644 --- a/cli/tests/application/display/cost-report-artefact.unit.test.ts +++ b/cli/tests/application/display/cost-report-artefact.unit.test.ts @@ -382,6 +382,7 @@ describe("buildCostReportArtefact — the flow axis states its own limits with t skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary" as const, }, ], }, diff --git a/cli/tests/domain/models/cost-report-order.property.unit.test.ts b/cli/tests/domain/models/cost-report-order.property.unit.test.ts index 50d65b0a..a199d947 100644 --- a/cli/tests/domain/models/cost-report-order.property.unit.test.ts +++ b/cli/tests/domain/models/cost-report-order.property.unit.test.ts @@ -60,6 +60,7 @@ const JOURNALS: readonly CostReportSessionJournal[] = [ skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-18T09:00:00Z"), endMs: Date.parse("2026-08-18T10:30:00Z"), + closedBy: "boundary", }, ], }, diff --git a/cli/tests/domain/models/cost-report.unit.test.ts b/cli/tests/domain/models/cost-report.unit.test.ts index b71c7b42..aacc8a5c 100644 --- a/cli/tests/domain/models/cost-report.unit.test.ts +++ b/cli/tests/domain/models/cost-report.unit.test.ts @@ -1184,11 +1184,13 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary", }, { skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T11:00:00Z"), endMs: Date.parse("2026-08-17T12:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1226,6 +1228,7 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T12:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1258,6 +1261,7 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1286,6 +1290,7 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1345,6 +1350,7 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T10:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1399,6 +1405,7 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1469,6 +1476,7 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary", }, ], }, @@ -1508,11 +1516,13 @@ describe("buildCostReport — by_flow reads the journal's own sequence, nothing skill: "aidd-orchestrator:01-sdlc", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T11:00:00Z"), + closedBy: "boundary", }, { skill: "aidd-orchestrator:02-backlog", startMs: Date.parse("2026-08-17T11:00:00Z"), endMs: Date.parse("2026-08-17T12:00:00Z"), + closedBy: "boundary", }, ], }, diff --git a/cli/tests/domain/models/flow-attribution.unit.test.ts b/cli/tests/domain/models/flow-attribution.unit.test.ts index 1386cd7a..64f05bff 100644 --- a/cli/tests/domain/models/flow-attribution.unit.test.ts +++ b/cli/tests/domain/models/flow-attribution.unit.test.ts @@ -96,11 +96,15 @@ describe("buildFlowIntervals — pure: journal lines -> bounded flow intervals", skill: SDLC_OPENS.skill, startMs: Date.parse(SDLC_OPENS.at), endMs: Date.parse(BACKLOG_OPENS.at), + closedBy: "boundary", }, { skill: BACKLOG_OPENS.skill, startMs: Date.parse(BACKLOG_OPENS.at), endMs: Date.parse(TURN_END.at), + // A pause never closes a flow, so this is the cap at the journal's last moment, + // which the pause happens to be. + closedBy: "journal-end", }, ]); }); @@ -113,6 +117,7 @@ describe("buildFlowIntervals — pure: journal lines -> bounded flow intervals", skill: SDLC_OPENS.skill, startMs: Date.parse(SDLC_OPENS.at), endMs: Date.parse(SDLC_ENDS.at), + closedBy: "boundary", }, ]); }); @@ -192,6 +197,7 @@ describe("buildFlowIntervals — pure: journal lines -> bounded flow intervals", skill: SDLC_OPENS.skill, startMs: Date.parse(SDLC_OPENS.at), endMs: Date.parse(TURN_END.at), + closedBy: "journal-end", }, ]); }); @@ -220,7 +226,12 @@ describe("buildFlowIntervals — pure: journal lines -> bounded flow intervals", const intervals = buildFlowIntervals(journalOf([bareSpelling, TURN_END])); expect(intervals).toEqual([ - { skill: "01-sdlc", startMs: Date.parse(bareSpelling.at), endMs: Date.parse(TURN_END.at) }, + { + skill: "01-sdlc", + startMs: Date.parse(bareSpelling.at), + endMs: Date.parse(TURN_END.at), + closedBy: "journal-end", + }, ]); }); @@ -232,6 +243,7 @@ describe("buildFlowIntervals — pure: journal lines -> bounded flow intervals", skill: SDLC_OPENS.skill, startMs: Date.parse(SDLC_OPENS.at), endMs: Date.parse(SDLC_OPENS.at), + closedBy: "journal-end", }, ]); }); @@ -307,6 +319,7 @@ describe("buildFlowIntervals — a journal with no readable moment in it", () => skill: SDLC_OPENS.skill, startMs: Date.parse(SDLC_OPENS.at), endMs: Date.parse(wroteLater.at), + closedBy: "journal-end", }, ]); }); diff --git a/cli/tests/domain/models/step-attribution.unit.test.ts b/cli/tests/domain/models/step-attribution.unit.test.ts index 24480af4..4daa4881 100644 --- a/cli/tests/domain/models/step-attribution.unit.test.ts +++ b/cli/tests/domain/models/step-attribution.unit.test.ts @@ -305,6 +305,75 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = }); }); + // An interval nothing ever closed ends at the journal's own last witnessed moment, which + // is a bound and not a measurement. Where one such interval sits inside another, the + // enclosing one answers: the inner one's end says only that the journal stopped, while + // the outer one is still known to have been open. Measured on the one orchestrated + // session captured, 2026-09-04: `aidd-dev:01-plan` opened at 06:00:50 inside an + // orchestration opened at 05:56:27, neither was ever closed, and reading the innermost + // start alone credited the invoked step with every one of the 972 records written over + // the three and a half hours that followed. + it("hands a moment to the orchestration when nothing ever closed the step inside it", () => { + const intervals = buildStepIntervals( + journalOf( + { type: "step_start", at: "2026-08-20T10:00:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "step_start", at: "2026-08-20T10:05:00Z", skill: "aidd-pm:04-spec" }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" } + ) + ); + + expect(attributeMoment(intervals, "2026-08-20T10:30:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-orchestrator:01-sdlc", + }); + }); + + // Two invoked steps in a row inside one orchestration. The first is closed by the + // second's own start, so its end is a witnessed boundary and it answers for the moments + // it covers; only the second is left unclosed, and it is the one that yields. There is + // never a tie between two unclosed invoked steps to break, because a `step_start` closes + // whichever plain step was open - which is what this case is here to demonstrate rather + // than assert in a comment. + it("keeps the earlier invoked step, and yields only the one nothing closed", () => { + const intervals = buildStepIntervals( + journalOf( + { type: "step_start", at: "2026-08-20T10:00:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "step_start", at: "2026-08-20T10:05:00Z", skill: "aidd-pm:04-spec" }, + { type: "step_start", at: "2026-08-20T10:20:00Z", skill: "aidd-dev:01-plan" }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" } + ) + ); + + expect(attributeMoment(intervals, "2026-08-20T10:10:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-pm:04-spec", + }); + expect(attributeMoment(intervals, "2026-08-20T10:30:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-orchestrator:01-sdlc", + }); + }); + + // The yielding is between two intervals nothing closed, and no wider than that. Here the + // orchestration states its own end while the step inside it does not, so the step runs + // past it and no interval encloses it - the innermost claim stands, exactly as it does + // when both ends are witnessed. + it("keeps the innermost step when the orchestration around it states its own end", () => { + const intervals = buildStepIntervals( + journalOf( + { type: "step_start", at: "2026-08-20T10:00:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "step_start", at: "2026-08-20T10:10:00Z", skill: "aidd-pm:04-spec" }, + { type: "step_end", at: "2026-08-20T10:20:00Z", skill: "aidd-orchestrator:01-sdlc" }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" } + ) + ); + + expect(attributeMoment(intervals, "2026-08-20T10:15:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-pm:04-spec", + }); + }); + // Nesting is declared, never inferred: only a skill `ORCHESTRATING_SKILLS` names invokes // others. Two ordinary skills in a row are a sequence, and the second still ends the first. it("still lets one ordinary step close another, which is a sequence and not a nesting", () => { @@ -360,6 +429,8 @@ describe("buildStepIntervals — a step the session never closed", () => { skill: "aidd-dev:01-plan", startMs: Date.parse("2026-08-17T10:00:00Z"), endMs: Date.parse("2026-08-17T12:00:00Z"), + // The cap, and named as one: nothing in this journal ever closed the step. + closedBy: "journal-end", }, ]); expect(attributeMoment(intervals, "2026-09-30T23:59:00Z")).toEqual({ source: "unattributed" });