Skip to content

Run an errors bulk action as one atom run, not one write per issue - #977

Merged
Makisuo merged 1 commit into
mainfrom
fix/errors-bulk-actions-one-run
Sep 21, 2026
Merged

Makisuo merged 1 commit into
mainfrom
fix/errors-bulk-actions-one-run

Conversation

@Makisuo

@Makisuo Makisuo commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Multi-selecting issues on /errors and applying a state, claim or severity only updated one issue, while the toast claimed every selected issue was updated.

Cause

The bulk actions fired one write per selected issue to the same per-endpoint mutation atom with Promise.all. An effect-atom function atom is not concurrent: each write refreshes it and interrupts the previous in-flight fiber, so only the last request reached the API. Every pending promiseExit promise then resolved with the atom's final result, so all N calls reported success.

Fix

  • issue-batch.ts adds forEachIssue, which runs a bulk action as one atom run with bounded concurrency, captures an Exit per issue, and invalidates the list once at the end instead of once per issue. batchOutcome folds those exits into counts for the toast.
  • use-issue-mutations.ts defines one batch atom each for transition, claim and severity on the API client runtime, and the three *Many functions call those. Partial failures now toast a warning with the real counts for all three actions.
  • issue-batch.test.ts covers per-issue exits in order, the concurrency bound, single invalidation after the batch, and the outcome folding.

Single-row actions are unchanged.

Verification

  • vitest run on the new test and the bulk bar test: 15 passed.
  • tsc --noEmit for apps/web: clean.
  • oxlint and oxfmt on the touched files: clean.
  • Not exercised in a browser: the local dev org has no seeded error issues.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features

    • Added bulk actions for transitioning, claiming, and updating the severity of multiple issues.
    • Bulk operations now report whether all, some, or none of the selected issues were updated.
    • Partial and complete failures provide clearer warnings and identify the first reported failure.
  • Bug Fixes

    • Issue lists and individual issue views now refresh consistently after bulk or single-issue updates.
    • Bulk actions continue processing other issues when one update fails.

…r issue

The bulk bar fired one write per selected issue to the same per-endpoint
mutation atom with Promise.all. That atom is not concurrent: each write
refreshes it and interrupts the previous fiber, so only the last request
reached the API. Every pending promiseExit then resolved with the atom's
final result, so all N calls reported success and the toast said "Moved N"
while one row changed.

Transition, claim and severity each get a batch atom on the API client
runtime. `forEachIssue` runs the ids with bounded concurrency, keeps an
Exit per issue so partial failures toast the real counts, and invalidates
the list once at the end instead of once per issue.
@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 341f43bf-ffe6-4f56-af01-d2d31b0db5de

📥 Commits

Reviewing files that changed from the base of the PR and between b5d9cb8 and 2d78ff2.

📒 Files selected for processing (3)
  • apps/web/src/components/errors/issue-batch.test.ts
  • apps/web/src/components/errors/issue-batch.ts
  • apps/web/src/components/errors/use-issue-mutations.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Adds shared utilities for bounded issue batching and failure aggregation. Applies them to bulk transitions, claims, and severity updates. Tests cover ordering, concurrency, invalidation, and batch outcomes.

Changes

Issue bulk mutations

Layer / File(s) Summary
Batch utilities and validation
apps/web/src/components/errors/issue-batch.ts, apps/web/src/components/errors/issue-batch.test.ts
Adds bounded issue processing, shared invalidation keys, outcome aggregation, and tests for concurrency, ordering, invalidation, and failures.
Batch mutation operations
apps/web/src/components/errors/use-issue-mutations.ts
Adds batch atoms for transitions, claims, and severity updates, and registers shared issue invalidation helpers.
Mutation outcome handling
apps/web/src/components/errors/use-issue-mutations.ts
Uses batchOutcome to handle complete success, partial success, and complete failure for transition, claim, and severity mutations.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant useIssueMutations
  participant BatchMutation
  participant forEachIssue
  participant batchOutcome
  useIssueMutations->>BatchMutation: submit issue IDs
  BatchMutation->>forEachIssue: run per-issue effects
  forEachIssue-->>BatchMutation: return ordered exits
  BatchMutation->>batchOutcome: aggregate exits
  batchOutcome-->>useIssueMutations: return counts and first failure
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: bulk error actions now run as one atom operation instead of one write per issue.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@Makisuo
Makisuo merged commit 49df32f into main Sep 21, 2026
40 checks passed
@Makisuo
Makisuo deleted the fix/errors-bulk-actions-one-run branch September 21, 2026 23:53
Makisuo added a commit that referenced this pull request Sep 22, 2026
… cost by 80%

Every review now writes one summary comment on the pull request and
edits it in place on later pushes, found again by a hidden
`<!-- maple-pr-review -->` line and only ever edited when the App wrote
it. It carries a score out of 100, a grade, the verdict, a counts table,
the summary, findings linked to their lines at the reviewed commit, what
to change, and what was reviewed. The check run shares the renderer and
leads its title with the score; the inline review now only carries the
notes and points at the comment.

The score is computed, not asked of the model: 100 minus 25 per
critical finding, 10 per warning and 2 per note, stated in the comment
footer and stored in pr_reviews.score (with comment_url beside it; the
unreleased migration is regenerated to include both). The verdict is
`gaps` exactly when a warning or critical finding is kept, which fixes a
check titled "0 observability gaps to close".

Iterating with the local runner cut a review of #962 from 388k input
tokens and 25 calls to 48k and 9, and #977 from 374k and 23 to 52k and
5, with the same verdicts. pr_changed_files states a call budget sized
to the reviewable files, the prompt reads the repository's conventions
before the diffs and only verifies what a finding depends on, it
forbids speculative findings and ending a message on a promise, and
tooling files are not reviewed. A local gap fixture with four planted
gaps came back with all four, on the right lines, in 5 calls.

The runner gains --range base..head for reviewing a local branch, --post
for writing the summary comment with your own gh login, and runs
sandbox_exec in a worktree at the head commit as production does.
Makisuo added a commit that referenced this pull request Sep 23, 2026
* feat(pr-review): review pull requests for observability gaps

A repository can opt in to having Maple review every pull request for
whether the code it adds will be visible in traces, logs and metrics.

The pull_request webhook now carries the head and base commits, and the
sink fans out to two isolated readers: the existing fix-verification
link and the new review trigger. PrReviewService writes a pr_reviews row
per head SHA, skips drafts, bots and duplicates, enforces a daily ceiling
per org, and starts one turn of the pr-review agent on a chat session
named after the review, the way an investigation's pass is started.

The agent runs under an allowlist of diff, source, sandbox and read-only
telemetry tools, reads the diff through two new internal tools backed by
the provider's pull request files endpoint (with new-side line numbers),
and files its report through submit_review, a lenient completion tool
normalized in the handler. The report is stored first, then posted as a
neutral or successful check run plus a comment-only review with inline
comments; a refused post is recorded on the row.

Settings get a per-repository switch and PUT
/api/integrations/github/repositories/:id/pr-review. The GitHub App needs
Checks and Pull requests write permissions, documented in
docs/github-app-setup.md. Migration 20260921100708_pr_reviews adds the
table and vcs_repositories.pr_review_enabled; production applies it by
hand.

* fix(pr-review): retry failed rows, drop stale submissions, reconcile the verdict

Review feedback on the first cut.

A redelivery for a head whose row is `failed` reclaims the row (a
conditional update from `failed` to `queued`) and starts it again,
instead of reporting a duplicate that could never retry. `submitReview`
and `failReview` now move a row only from `queued` or `running`, so a
review superseded while its completion call was in flight stays
`skipped` and never posts stale findings.

A retained warn or critical finding forces the `gaps` verdict whatever
the model called it, so the check run's conclusion and its inline
comments cannot disagree. The check summary escapes backslashes before
pipes and is cut under GitHub's 65,535-character limit.

Also: the switch gets an accessible name on narrow screens, the
anticipated error identifiers include the review not-found error, the
dispatcher test lists the two new internal tools, and both docs match
what shipped.

* fix(pr-review): purge pr_reviews with the organization

The org-scoped table registry pins every table carrying an org column;
pr_reviews belongs with the VCS tables that are deleted by org_id.

* fix(pr-review): clamp the check summary by UTF-8 bytes

GitHub's 65,535 limit on output.summary is bytes, not characters, so a
report in a multi-byte script could pass the character count and be
refused, losing the review post with it. The clamp now budgets encoded
bytes, notice included, and cuts on a code point boundary.

* fix(pr-review): keep the review switch's label visible at every width

The visible label was hidden below the sm breakpoint, leaving sighted
users an unlabeled switch; the aria-label only covered assistive tech.

* feat(pr-review): run the reviewer locally against any pull request

`bun run --cwd apps/ai review:local <owner/repo> <number>` drives the
real pr-review agent (prompt, budget, allowlist, engine loop, model,
submit_review and buildPublication) against a real pull request fetched
with the caller's gh login. The diff tools print through the same
renderers production uses, now pure functions over the PR's files. The
source tools read a local clone at the head commit through git,
sandbox_exec is limited to read-only git, and the telemetry tools say no
warehouse is attached. Nothing is posted; each run writes review.md (the
check run and comments as GitHub would render them, with findings off
the diff flagged), transcript.md and report.json. --prompt-file swaps
the system prompt for one run through a new promptOverride on the run
input, and --model picks the OpenRouter model.

The first real run found a production bug: sandbox_grep ran git grep in
basic-regex mode, so a model's `a|b` matched a literal bar and `\(`
failed outright. The reviewer concluded the web app had no telemetry.
It now uses extended regex, pinned by a test against a real repository.

* fix(pr-review): match the local clone's remote without building a regex from arguments

The owner and repository came from the command line and were
interpolated into a RegExp. The remote URL is now parsed with a fixed
pattern and its parts compared as strings.

* feat(pr-review): always post a scored summary comment, and cut review cost by 80%

Every review now writes one summary comment on the pull request and
edits it in place on later pushes, found again by a hidden
`<!-- maple-pr-review -->` line and only ever edited when the App wrote
it. It carries a score out of 100, a grade, the verdict, a counts table,
the summary, findings linked to their lines at the reviewed commit, what
to change, and what was reviewed. The check run shares the renderer and
leads its title with the score; the inline review now only carries the
notes and points at the comment.

The score is computed, not asked of the model: 100 minus 25 per
critical finding, 10 per warning and 2 per note, stated in the comment
footer and stored in pr_reviews.score (with comment_url beside it; the
unreleased migration is regenerated to include both). The verdict is
`gaps` exactly when a warning or critical finding is kept, which fixes a
check titled "0 observability gaps to close".

Iterating with the local runner cut a review of #962 from 388k input
tokens and 25 calls to 48k and 9, and #977 from 374k and 23 to 52k and
5, with the same verdicts. pr_changed_files states a call budget sized
to the reviewable files, the prompt reads the repository's conventions
before the diffs and only verifies what a finding depends on, it
forbids speculative findings and ending a message on a promise, and
tooling files are not reviewed. A local gap fixture with four planted
gaps came back with all four, on the right lines, in 5 calls.

The runner gains --range base..head for reviewing a local branch, --post
for writing the summary comment with your own gh login, and runs
sandbox_exec in a worktree at the head commit as production does.

* fix(pr-review): read diffs in batches so a large pull request fits its budget

A 33-file review read one pr_file_diff per call; every call re-sends the
conversation, so it reached 724k input tokens, ran out of budget and was
killed for asking for another diff. pr_file_diff now takes several paths
per call, up to 60k characters, and names what did not fit. The same
review now takes 11 calls and 402k tokens and completes.

The local runner also runs the close-out after a failed pass, as the
turn runner does in production, unless the provider itself failed.

* feat(pr-review): stage the rollout per organization behind the prreview flag

Merging the feature now turns it on for no one. An organization is
flagged in with `prreview: true` in its Clerk public metadata, the same
place as the other rollout flags.

The flag contract moves from apps/web into @maple/domain so the server
decodes the same metadata the web app does, and a new
OrganizationFeatureFlagsService reads it from Clerk (every rollout on
for self-hosted builds, off when Clerk cannot be reached). The switch
in Integrations → GitHub appears only for a flagged organization, the
settings endpoint refuses to turn reviews on for an unflagged one, and
the review trigger skips its pull requests as not_rolled_out, so
withdrawing the flag stops reviews on repositories already switched on.

* fix(pr-review): post the review even when the installation cannot write check runs

The production App registers checks: read, so every installation today
answers 403 to a check run, and the check run was created first: the
whole publication failed and nothing reached the pull request. A 403 on
the check run is now recorded on the span and the summary comment and
inline notes are posted anyway, since they need only pull_requests:
write.

* docs(github-app): the Checks permission is optional for reviews

* refactor(pr-review): cleanup pass from review

- Trigger: the daily ceiling counts every row started today; state
  transitions are guarded by the statuses they leave, so a superseded
  row cannot be flipped back to running or failed.
- submitReview resolves the repository and installation before marking
  the row completed, so a missing one lands as publish_error in the same
  update.
- Publishing: a rate-limited 403 no longer reads as a missing checks
  permission; a 422 on the inline review drops the notes instead of
  re-posting an empty review under the summary comment.
- Findings need a maple-audit check id; an unknown id is dropped rather
  than defaulted to SPAN-02. A gap is never graded excellent.
- submit_review is offered to the unattended pass only; a follow-up in
  the review session answers in prose.
- Review turns bill as source "review" with their own idempotency key.
- The close-out transcript is shared (chat/close-out.ts) and the local
  runner uses it; the runner's prompt override is an agent override on
  ChatRunInput, its telemetry stubs derive from PR_REVIEW_TOOLS, and its
  sandbox_exec refuses git flags that reach outside the object store.
- OrganizationFeatureFlagsService caches an org's flags for 60s.
- Dead PullRequestEventSinkLive export and unused tool param removed;
  prReviewSessionId lives in @maple/domain/chat-session.
- docs/pr-review-agent-plan.md rewritten as a present-tense design doc.

* feat(vcs): give the pull request review publish its own span

The check run, summary comment and inline review posts ran under the
caller's span, so the skip and rejection annotations landed there. The
review's own summary flagged it as MAP-02.
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