Skip to content

fix(comment): bound errored-project rendering to prevent comment-generation hang - #21

Merged
liamg merged 2 commits into
mainfrom
fix-errored-project-comment-hang
Jul 2, 2026
Merged

fix(comment): bound errored-project rendering to prevent comment-generation hang#21
liamg merged 2 commits into
mainfrom
fix-errored-project-comment-hang

Conversation

@liamg

@liamg liamg commented Jul 2, 2026

Copy link
Copy Markdown
Member

Problem

A production run hung indefinitely after logging Total after diff: 182 project(s). A goroutine dump showed the single run goroutine [runnable] (on-CPU, not blocked on I/O or locks) spinning in:

comment.formatErroredProject      cost_details.go
→ processProjectCostDetails
→ Render / GenerateComment

formatErroredProject built its output with s += ... while splitting the diagnostic message on ": " and applying a per-piece growing indent via strings.Repeat. A large critical diagnostic — e.g. a multi-MB Terraform HCL parse error, which also contains many ": " separators — turns this into an O(n²) string build (quadratic in both concatenation and cumulative indent). Because this runs inside GenerateComment, before renderWithTruncation, the 64 KB comment cap never gets a chance to kick in.

Fix

pkg/vcs/comment/cost_details.go:

  • formatErroredProject: use strings.Builder; cap each diagnostic message to 4 KB (rune-safe, via prefixWithin) before splitting; cap indent depth at 8; cap the number of critical diagnostics rendered per project at 10, with a … and N more error(s) summary.
  • processProjectCostDetails: cap the combined errored-projects section at 32 KB (built before truncation), with a … and N more project(s) with errors note.

Both quadratic terms are removed; total work is now linear and bounded regardless of diagnostic size or error count.

Test

Adds errored_project_test.go:

  • TestFormatErroredProject_Pathological — a ~6 MB diagnostic with ~2M ": " separators. Verified it hangs (5s guard trips, ~4 cores pinned) on the old implementation and completes in ~0s with the fix, output bounded.
  • TestFormatErroredProject_LimitsErrorCount — asserts the per-project error cap + summary.

go build ./..., go vet, and the full pkg/vcs/comment suite pass.

liamg added 2 commits July 2, 2026 20:08
…ration hang

formatErroredProject built its output with `s += ...` while splitting the
diagnostic message on ": " and applying a per-piece growing indent via
strings.Repeat. A large critical diagnostic (e.g. a multi-MB Terraform HCL
parse error, which also contains many ": " separators) turned this into an
O(n^2) string build, spinning a single goroutine on-CPU indefinitely. This
runs inside GenerateComment, before renderWithTruncation, so the 64KB comment
cap never kicked in.

Fixes:
- formatErroredProject uses strings.Builder, caps each diagnostic message to
  4KB before splitting, caps indent depth, and caps the number of critical
  diagnostics rendered per project (10) with a summarised remainder.
- processProjectCostDetails caps the combined errored-projects section (32KB),
  since the whole section is built before truncation.

Adds a regression test that hangs on the old implementation and completes
instantly with the fix.
@liamg
liamg merged commit 96f514d into main Jul 2, 2026
3 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.

2 participants