Skip to content

fix(output): make rendering cancellable mid-stream - #26

Merged
hammadmajid merged 1 commit into
fix/13-gitengine-contextfrom
fix/17-output-cancellation
Sep 5, 2026
Merged

hammadmajid merged 1 commit into
fix/13-gitengine-contextfrom
fix/17-output-cancellation

Conversation

@hammadmajid

@hammadmajid hammadmajid commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Description

After a single pre-flight check, a multi-million-line result set streamed to stdout with no cancellation. Into a slow pipe, Ctrl-C was inert for the whole render.

Closes #17

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Performance improvement
  • Refactoring or code cleanup
  • Documentation update
  • CI/CD or build workflow change

Key Changes

Formatter.Format gained a leading ctx context.Context, threaded into all four concrete formatters (GroupedFormatter, SingleLineFormatter, CountFormatter, FilesWithMatchesFormatter).

An unexported cancelGuard holds the context plus a plain int line budget. boundary() forces a ctx.Err() read at each file-group boundary and restarts the interval; lines(n) decrements the budget and only reads the context once every 1024 lines. Per-line cost is a decrement and a branch, so the atomic load stays off the innermost write path. A nil context degrades to context.Background().

Guard checks sit between line writes, so whatever was emitted before an abort is always a whole-line prefix of the complete render — never a truncated line.

In emitResults the now-redundant pre-flight check is folded into the render, and a cancellation return maps to cancelError using the same predicate already used at the walker.Walk call site.

Verification & Testing

  • Ran go test -v -count=1 ./...
  • Ran go test -race -shuffle=on -count=1 ./...
  • Ran go vet ./...
  • Added or updated unit/integration tests
  • Tested manually against sample Git repository histories
go build ./...                                          clean
go vet ./internal/output/... ./cmd/...                  clean
go test -race -count=1 ./internal/output/... ./cmd/...  ok 1.25s / 1.21s

End-to-end proof the fix is load-bearing. 34,962-line render into a deliberately slow pipe (~500 lines/sec consumer), SIGINT after 1 s:

Binary Behaviour
Pre-fix (HEAD) still alive > 5 s later, signal inert
Post-fix exits 0.319 s after SIGINT, having delivered 285 of 34,962 lines

The 0.3 s is the blocked write(2) draining the 64 KB pipe buffer, after which the guard observes the cancellation.

New tests. TestFormatterCancelledMidRender tables over all four formatters, asserts the fixture spans more than two check intervals, then re-renders through a writer that cancels after 10 emitted lines — asserting errors.Is(err, context.Canceled), non-empty output, strictly shorter than the full render, a strings.HasPrefix of it, and newline-terminated. TestFormatterAlreadyCancelledEmitsNothing defends the folded pre-flight check.

Mutation: neutering the guard's ctx.Err() reads fails all 5 subtests. Separately, mutating CountFormatter to observe cancellation but still emit everything fails on cancellation emitted 78000 of 78000 bytes: the remainder was not skipped — proving the strict-prefix and length assertions have teeth independently of the error assertion.

Note

Stacked on #25.

Checklist

  • gofmt clean
  • Every new test mutation-checked: reverting the fix makes it fail
  • This layer builds and passes the full race suite on its own, not just at the top of the stack

Formatter.Format took no context, so after emitResults' single pre-flight
ctx.Err() check a multi-million-line result set streamed to stdout with no
way to stop it. If stdout was a slow pipe, Ctrl-C was inert for the entire
render.

Add a leading ctx to the Formatter interface and thread it through all four
concrete formatters (grouped, single-line, count, files-with-matches). Each
consults the context through a cancelGuard, which reads it at file-group
boundaries and once every 1024 emitted lines; the per-line cost is a
decrement and a branch, keeping the atomic load off the innermost write
path. Checks sit between line writes, so the output produced before an abort
is always a whole-line prefix of the complete rendering.

In emitResults the now-redundant pre-flight check is folded into the render
itself, and a cancellation return maps to cancelError using the same
predicate as the walker call site above it.

Also updates test/leak_test.go's single Format call site to the new
signature (agreed with the lead; no agent owns that file in this wave).

Verified end-to-end: rendering 34962 lines into a slow pipe, SIGINT is
ignored for >5s before this change and aborts after 0.32s with only 285
lines emitted after it.

Closes #17
@hammadmajid
hammadmajid force-pushed the fix/17-output-cancellation branch from a7037ae to e485a09 Compare September 5, 2026 14:16
@hammadmajid
hammadmajid marked this pull request as ready for review September 5, 2026 14:20
@hammadmajid
hammadmajid merged commit 22a2c9f into main Sep 5, 2026
3 of 6 checks passed
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.

output: rendering is not cancellable once it starts

1 participant