Skip to content

fix(review): improve large-target review flow - #1554

Merged
limityan merged 1 commit into
GCWing:mainfrom
limityan:yanzhn/review-managed-batching
Jul 15, 2026
Merged

fix(review): improve large-target review flow#1554
limityan merged 1 commit into
GCWing:mainfrom
limityan:yanzhn/review-managed-batching

Conversation

@limityan

Copy link
Copy Markdown
Collaborator

Summary

  • keep slash-command suggestions available when / is entered at the start of an input that already has trailing text
  • replace the large Review target hard failure with bounded managed batching and explicit partial coverage
  • keep ordinary Review as the only product-facing mental model: no routine confirmation dialog, no internal worker/tool/packet names
  • project background review tasks from the real child lifecycle instead of the spawn acknowledgement

Behavior change

Scenario Before After
Slash at input start with existing text no command suggestions command picker opens and preserves trailing text
Ordinary Review up to 80 prepared files one read-only reviewer unchanged
Larger local target hard rejection around the evidence limit up to 8 batches x 40 files, at most 2 concurrent workers, 120s worker timeout, explicit deferred scope
Provider PR target hard rejection above 500 files collect up to 1,000 file entries; plan at most 128 provider-backed files per turn and prefer complete evidence
Managed execution UX internal worker/tool details could surface one Review flow with coverage, parallelism, and deferred-count summary
Background Task lifecycle spawn acknowledgement could look completed in ~100ms card follows child running/completed/error/cancelled state and child duration

launch_batch is documented as a preferred capacity grouping, while the runtime guarantee remains the global concurrency ceiling. This avoids introducing a second scheduler or workflow FSM.

Safety and scope

  • managed workers remain read-only and use only GetFileDiff, Read, Grep, Glob, and LS
  • runtime validates packet ids, file scopes, duplicate assignments, batch/concurrency/time bounds, and reviewer-call ceilings
  • omitted or unplanned files remain explicit deferred coverage; the final report cannot present partial evidence as full approval
  • no new product page, worker console, public event contract, or confirmation step was added

Independent review

Independent senior architecture and product-design reviews were run against the full change. Their findings were fixed and re-reviewed:

  • aligned provider PR planned coverage with the 128 per-turn acquisition ceiling
  • corrected the launch_batch contract without adding a scheduler
  • unified LaunchReviewAgent into the existing Review coverage card
  • projected child error/cancelled/completed states and added a fine-grained child-session subscription
  • added a non-blocking managed coverage summary to the existing Review action bar

Both final reviews reported no remaining P0-P2 findings and no boundary expansion.

Validation

  • pnpm --dir src/web-ui run test:run ... — 12 files, 187 tests passed
  • pnpm run type-check:web
  • pnpm run i18n:contract:test
  • pnpm run i18n:audit — 0 warnings
  • pnpm run check:repo-hygiene
  • pnpm run check:github-config
  • rustup run nightly cargo check --workspace
  • focused bitfun-agent-runtime, bitfun-core, and bitfun-services-integrations --features review-platform tests
  • git diff --check

The workspace check retains pre-existing Windows unsafe_op_in_unsafe_fn warnings outside this change.

AI-assisted implementation; all listed checks were run locally.

@limityan
limityan merged commit 8921ea8 into GCWing:main Jul 15, 2026
5 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.

1 participant