refactor(render-payload): migrate five payload lanes to Outcome, one boundary log site (2l6.1)#104
Merged
Conversation
…boundary log site (2l6.1) Every provider lane (git/cache/session/today/context/metrics/tmux) now carries Outcome(ok|absent|failed); buildRenderPayload is the single log site [LAW:one-type-per-behavior][LAW:single-enforcer][LAW:no-silent-failure]. - session: getSessionUsageFromPath returns Outcome; toSessionInfo takes non-null usage; store ingest/aggregate carry failure, failed parses are not cached (only ok records enter the map, same rule as the git cache) - today: TodayInfo tightened to non-null totals; "no usage today" is absent; a failed seed or active-session ingest fails the projection loudly instead of summing a wrong total; seed logs its own per-session parse failures (its own effect edge) - context: unreadable transcript is failed, no-usable-entry is absent - metrics: no-cost hook is absent; MetricsInfo tightened (only lastResponseTime stays nullable) - tmux: cache stores the resolved Outcome; spawn failure stays durable but visible as failed at the boundary instead of demoted to null - parseJsonlFile: ENOENT stays [] (new session pre-first-write); every other read error propagates so providers classify it as failed — the old catch-all-to-[] dressed EACCES/EIO as an empty session - boundary: one generic lane() helper replaces the per-lane null plumbing and casts; one pure take() fold; doc comment collapses to one sentence
brandon-fryslie
commented
Jun 12, 2026
brandon-fryslie
left a comment
Contributor
Author
There was a problem hiding this comment.
Adversarial review
This is a well-executed refactor that correctly unifies seven provider lanes from two competing failure-visibility contracts into one Outcome-typed boundary. The lane/take helpers are correctly implemented, exception safety via .catch is preserved for all async paths, TodayInfo/MetricsInfo type tightening is correctly reflected at every usage site, and the parseJsonlFile ENOENT/non-ENOENT split is semantically sound. The test suite is thorough and the new boundary tests pin the unified contract precisely. No correctness defects found.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes brandon-git-segment-2l6.1 (follow-up surfaced by PR #96).
What
buildRenderPayloadhad two failure-visibility contracts side by side: the git/cache lanes carried typedOutcomes with boundary logging, while session/today/context/metrics/tmux still.catch(() => null)and relied on interiordebug()logging — two types for one behavior at one boundary ([LAW:one-type-per-behavior]).All seven lanes now carry
Outcome(ok | absent | failed); the boundary is the single log site ([LAW:single-enforcer],[LAW:no-silent-failure]); the doc comment collapsed to one sentence as the ticket specified.Per-lane classification (where the semantics are known)
getSessionUsageFromPathreturnsOutcome; failed parses are NOT cached in the store (only ok records enter the map — same rule as the git cache), so the next render retries.TodayInfotightened to non-null totals ([LAW:types-are-the-program]); "no usage today" isabsent; a failed seed or failed active-session ingest fails the projection loudly instead of summing a confidently-wrong total. The once-a-day seed logs its own per-session parse failures (it is its own effect edge — no render boundary to carry outcomes to).failed; transcript with no usable usage entry isabsent.absent;MetricsInfotightened so onlylastResponseTimestays nullable (the one field whose domain genuinely has no answer sometimes).Outcome; a spawn failure stays durable (no re-spawn per render) but flows to the boundary asfailedinstead of being demoted to null.The deeper seam
parseJsonlFileswallowed all read errors to[]— with that in place the newfailedarms were unreachable for real read errors. Now ENOENT stays[](a new session pre-first-write is the domain's genuine "no entries"), and every other error (EACCES, EIO) propagates so the consuming provider classifies it. All three callers are the providers migrated here.Boundary cleanup
One generic
lane()helper replaces the per-lane null plumbing — the fivenullP<Awaited<ReturnType<...>>>()incantations and bothas Promise<Outcome<...>>casts disappeared because the types converged. One puretake()fold projects single-value lanes; the log effect happens once at the edge ([LAW:effects-at-boundaries]).Verification
pnpm typecheck,pnpm lint,pnpm check:protocol,pnpm buildall cleanfailed(chmod 000), missing transcript → graceful zero-entry, no-cost hook →absentNote
The z.ai reviewer account is 429-blocked (account-level, restoration requested) — the Code Review check will fail until restored; this is expected and not caused by this PR.