Skip to content

docs(ticketmill): document pre-pool byStage invariant at aggregateTokens reconciles line - #190

Merged
aaddrick merged 1 commit into
Batch_2026-08-01_215812from
issue-120-aggregatetokens-add-point-of-risk-comment-tying-an
Aug 2, 2026
Merged

docs(ticketmill): document pre-pool byStage invariant at aggregateTokens reconciles line#190
aaddrick merged 1 commit into
Batch_2026-08-01_215812from
issue-120-aggregatetokens-add-point-of-risk-comment-tying-an

Conversation

@aaddrick

@aaddrick aaddrick commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Closes #120

Summary

Adds a terse 3-5 line // comment immediately above the reconciles definition in aggregateTokens() (workflows/ticketmill.js, ~line 2297) documenting the point-of-risk invariant flagged during issue #65's review (fixed via PR #119) but never folded into the shipped fix.

The comment records:

  • Invariant: every byStage bucket is sampled outside runPool(), the only concurrent region, via addStage()/STAGE_TOKENS bracketing — making byStage exact at any CONCURRENCY.
  • Consequence: this is the only reason !anyTracked (stage-buckets-only breakdown) may safely claim reconciles: true above concurrency 1.
  • Obligation: any future byStage bucket sampled inside the pool must revisit this line or reintroduce issue aggregateTokens: reconciles flag mislabels stage-only-attributed breakdowns as approximate at concurrency>1 #65's false "reconciles exactly" caveat.

.claude/workflows/ticketmill.js was regenerated byte-identically via node scripts/lint-engine.js --fix in the same commit.

Key decisions

  • Strictly documentary change — no logic modified; verified via git diff -U0 showing only added // lines.
  • Kept the comment terse given the engine's 512 KiB launch cap (~478 KB, under lint-engine's 92% warn band).
  • No prose duplicated into docs/architecture/engine-internals.md — the docs already carry the invariant (module header, addStage(), STAGE_LABELS); the gap was the missing local warning at the point of risk.
  • Verified: node --check, node scripts/lint-engine.js (clean, lockstep confirmed via cmp), bare node --test (781/781 green, incl. token-reconcile/token-usage/sandbox-lint/architecture-provenance).

Token usage (approximate, this issue only): 71654 output tokens

#120)

Insert a terse comment above the reconciles line in aggregateTokens()
recording (a) the invariant that every byStage bucket is sampled outside
runPool() via addStage()/STAGE_TOKENS bracketing, exact at any
CONCURRENCY, (b) the consequence that this is the only reason
!anyTracked may claim reconciles:true above concurrency 1 (issue #65,
fixed by #119), and (c) the obligation on anyone adding a byStage
bucket sampled inside the pool to revisit this line. Synced
.claude/workflows/ticketmill.js via node scripts/lint-engine.js --fix.

No executable logic changed.
@aaddrick

aaddrick commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Spec Review (Iteration 1)

Verdict: approved

Issue #120 asked for a terse point-of-risk comment above the reconciles definition in aggregateTokens() documenting the pre-pool byStage invariant, its consequence for !anyTracked at concurrency > 1, and the obligation for future in-pool byStage buckets — with lockstep sync to .claude/workflows/ticketmill.js.

Verified against the diff (origin/Batch_2026-08-01_215812...HEAD):

  • Only two files touched: workflows/ticketmill.js and its mirror .claude/workflows/ticketmill.js, +6/+6 lines, both identical additions.
  • The inserted comment covers all three required elements: (a) invariant — byStage buckets sampled outside runPool() via addStage()/STAGE_TOKENS bracketing; (b) consequence — the only reason !anyTracked may claim reconciles:true above concurrency 1; (c) obligation — revisit if a future byStage bucket is sampled inside the pool. Cross-references issue aggregateTokens: reconciles flag mislabels stage-only-attributed breakdowns as approximate at concurrency>1 #65/fix(engine): aggregateTokens reconciles flag no longer mislabels stage-only concurrency>1 breakdowns #119 by name as required.
  • No executable logic changed — purely a comment insertion, confirmed by diff inspection.
  • No scope creep: no runtime guards, no test additions for hypothetical states, no docs moved to engine-internals.md — matching the explicitly rejected alternatives in the adjudicated approach.
  • Engine byte-size and lockstep sync verified fine per task/quality-review evidence (478,838/524,288 bytes, under the 482,344 warn band).

No missing acceptance criteria; no scope-creep concerns. Approving with no required changes.

@aaddrick

aaddrick commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Code Review (Iteration 1)

Verdict: approved — no blocking, major, or minor findings.

Diff reviewed: origin/Batch_2026-08-01_215812...HEAD (a85dc43), 12 added lines across workflows/ticketmill.js and its lockstep mirror .claude/workflows/ticketmill.js, all of them // comment lines.

Correctness of the comment's claims (verified against the code, not taken on trust):

  • STAGE_TOKENS is a plain literal at :1656 with exactly two buckets, populated only through addStage() (:1725).
  • All four addStage() call sites (:7172, :7274, :7456, :7490) sit in the top-level run body and are ordered strictly before runPool()'s drain (:5268 definition; drain sequenced after :7511's guard log), so the "sampled outside the only concurrent region" invariant holds today.
  • The consequence claim matches the expression verbatim: reconciles = hasSpent && trackedAny && (concurrency === 1 || !anyTracked) (:2303) — !anyTracked is indeed the only disjunct that can be true above concurrency 1, and it is only reachable when the breakdown is stage-buckets-only.
  • The obligation clause is the right hazard to name: STAGE_LABELS' unknown-key fallback (:2285) explicitly anticipates new buckets, so the invariant is load-bearing and was previously invisible at this line.

Persona checklist:

  • Sandbox compliance: no Date.now(), Math.random(), argless new Date(), fs/Node API, or TS syntax introduced — the change is comment-only; node scripts/lint-engine.js exits 0 clean.
  • Orchestration invariants / caps / circuit breakers: untouched; no MAX_*, stage order, or pipeline-shape change, so no docs or D2 diagram drift obligation.
  • Prompt/schema coherence, agentType, VERIFY_SKIPS, incident machinery (stub-task guard, settled-decisions ledger, handoff notes, comment markers, claim label-safety, browser lock, degrade windows): none touched.
  • Release discipline: no marketplace.json/version/CHANGELOG changes in this per-issue diff — correct, those are batch-level.
  • Lockstep + size: cmp confirms the two engine copies are byte-identical at 478,838/524,288 bytes, under lint-engine's 482,344-byte warn band; the +6 lines cost ~400 bytes and leave headroom intact.
  • Full suite green: node --check clean, bare node --test 781/781 pass.

The comment does not duplicate the existing prose at :1650-1656 and :1715-1724 — it points at it from the point of risk, which is exactly the gap issue #120 describes.

@aaddrick
aaddrick merged commit 22d4fbb into Batch_2026-08-01_215812 Aug 2, 2026
1 check passed
@aaddrick
aaddrick deleted the issue-120-aggregatetokens-add-point-of-risk-comment-tying-an branch August 2, 2026 02:19
@aaddrick

aaddrick commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Implementation Complete

Branch issue-120-aggregatetokens-add-point-of-risk-comment-tying-an has been squash-merged into Batch_2026-08-01_215812.

  • Spec review: passed
  • Code review: passed
  • Merge: squash, branch deleted

No deferred suggestions were collected during implementation, so there is no follow-up section for this PR.

aaddrick added a commit that referenced this pull request Aug 9, 2026
* docs(engine): note pre-pool byStage invariant at reconciles definition (#120) (#190)

Insert a terse comment above the reconciles line in aggregateTokens()
recording (a) the invariant that every byStage bucket is sampled outside
runPool() via addStage()/STAGE_TOKENS bracketing, exact at any
CONCURRENCY, (b) the consequence that this is the only reason
!anyTracked may claim reconciles:true above concurrency 1 (issue #65,
fixed by #119), and (c) the obligation on anyone adding a byStage
bucket sampled inside the pool to revisit this line. Synced
.claude/workflows/ticketmill.js via node scripts/lint-engine.js --fix.

No executable logic changed.

* chore(release): v0.2.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant