Skip to content

feat(cli): check which chunks the server needs before uploading - #98154

Merged
trunk-io[bot] merged 3 commits into
masterfrom
posthog/cli-symbol-set-upload-check
Sep 11, 2026
Merged

trunk-io[bot] merged 3 commits into
masterfrom
posthog/cli-symbol-set-upload-check

Conversation

@ablaszkiewicz

@ablaszkiewicz ablaszkiewicz commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A symbol set upload makes two API calls per batch of 50 chunks, even when the server already holds every chunk. A build with 3600 unchanged chunks makes 144 calls to upload nothing.

Changes

  • The CLI now asks the server which chunks it still needs, then uploads only those.
  • It sends the chunk ids and content hashes to bulk_check_upload (feat(error-tracking): add a symbol set upload check endpoint #98153), 1000 chunks per request.
  • A batch that uploads no chunk skips the finish call.
  • The CLI falls back to the old flow when the server rejects the check, so it works against a server without the endpoint.
  • Mechanical: the content hashes are computed once for all chunks instead of once per batch.

API calls for a synthetic build of 3600 chunks against the local dev stack:

Run Before After
Nothing changed 144 4
180 chunks changed 144 12
First upload 144 148

How did you test this code?

  • The table above comes from sourcemap process --release-mode=event on a synthetic build. The web-raw example app gave the same result in both release modes.
  • A run against a server without the endpoint confirmed the fallback.
  • A unit test drives the check with scripted replies: an answer after two server failures, three server failures, and a 403 or a 400 on the first attempt. It catches a change that stops retrying, or that retries a rejection.

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Claude Code with Claude Fable 5.1. Skills invoked: /writing-tests, /writing-code-comments, /writing-user-facing-copy, /writing-pr-descriptions, /asd-ste100. An old server rejects the unknown action with 403 for a personal API key, not 404, so the fallback covers every rejection.

🤖 Generated with Claude Code

https://claude.ai/code/session_015aB7hMSfqghiJRCpHienaF

The CLI sends the chunk ids and content hashes to bulk_check_upload
first, 1000 chunks per request, and then uploads only the chunks the
server names. A batch that uploads nothing skips the finish call. The
CLI falls back to the old flow when the server rejects the check.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aB7hMSfqghiJRCpHienaF
@ablaszkiewicz ablaszkiewicz self-assigned this Sep 10, 2026
@trunk-io

trunk-io Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

🚨 Comment density — 9% of added code lines are comments (22 of 245)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
cli/src/api/symbol_sets.rs 22 245

This check does not block merging. It updates on every push and clears when the share drops.

@posthog

posthog Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 0 should fix, 1 consider.

Published 1 finding (view the review).

Resolved comments: 1 fixed

@posthog

posthog Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review

Found 1 consider.

Comment thread cli/src/api/symbol_sets.rs Resolved
A check that gave no usable answer discarded every chunk id that an earlier
1000-chunk batch had already cleared, so all of them went back into the start
batches. Keep the answered batches' results and send only the unanswered
batches without a check.

Generated-By: PostHog Desktop
Task-Id: e9ef07a7-53aa-4a8f-88a4-920a8062bdc8
@ablaszkiewicz
ablaszkiewicz marked this pull request as ready for review September 10, 2026 09:25
Copilot AI lite review requested due to automatic review settings September 10, 2026 09:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
cli/src/api/symbol_sets.rs:477-484
**Retry flow lacks coverage**

The new upload-check path retries transport and server failures up to three times, but the added test only checks filtering after a failed callback. This violates the repository requirement to test complete retry flows with multiple attempts. Add coverage for eventual success and for retry exhaustion falling back to the original upload flow before merging.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(cli): keep upload check results when..." | Re-trigger Greptile

Comment thread cli/src/api/symbol_sets.rs Resolved
The retry loop and the outcome classification of the upload check move
into check_upload_with, which takes the delays and a send closure. A
unit test drives it with scripted replies.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aB7hMSfqghiJRCpHienaF

@cat-ph cat-ph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 💪

@trunk-io
trunk-io Bot merged commit 342f1f1 into master Sep 11, 2026
201 checks passed
@trunk-io
trunk-io Bot deleted the posthog/cli-symbol-set-upload-check branch September 11, 2026 12:29
@deployment-status-posthog

deployment-status-posthog Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-09-11 12:56 UTC Run
prod-us ✅ Deployed 2026-09-11 13:08 UTC Run
prod-eu ✅ Deployed 2026-09-11 13:12 UTC Run

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.

3 participants