Skip to content

Replace N+1 execution fetches in batch-status handler with one batch fetch#811

Open
AbirAbbas wants to merge 1 commit into
mainfrom
issue/417f3831-batch-status-n-plus-1
Open

Replace N+1 execution fetches in batch-status handler with one batch fetch#811
AbirAbbas wants to merge 1 commit into
mainfrom
issue/417f3831-batch-status-n-plus-1

Conversation

@AbirAbbas

Copy link
Copy Markdown
Contributor

Summary

  • Replaces N+1 per-ID storage fetches in handleBatchStatus with a single GetExecutionRecordsBatch query using a parameterized WHERE execution_id IN (...) clause.
  • Preserves the per-ID not_found response contract for missing execution IDs while collapsing all DB round-trips to one.
  • Rejects oversized batches (>500 IDs, 400 on the oversize path) with a 400 before any storage call, and reuses the batched webhook-registration lookup to avoid a secondary N+1.

Changes

  • control-plane/internal/storage/storage.go: adds GetExecutionRecordsBatch to the StorageProvider/ExecutionStore interfaces.
  • control-plane/internal/storage/execution_records.go: implements GetExecutionRecordsBatch on LocalStorage with a single parameterized IN (...) query.
  • control-plane/internal/handlers/execute.go: rewrites handleBatchStatus to fetch once and enforce the batch-size cap.
  • Test stubs across internal/handlers/* and internal/server/* updated to satisfy the new storage interface; new execute_batch_status_test.go and execution_records_test.go cover the batch path, per-ID not_found mapping, and oversize rejection.

Test plan

  • cd control-plane && go vet ./... (clean).
  • cd control-plane && go test ./internal/handlers/... ./internal/storage/... (passing).
  • Confirm a 500-ID batch triggers exactly one storage call and that oversize batches (e.g. 501 IDs) return 400 without hitting storage.

Closes #430

🤖 Built with AgentField SWE-AF
🔌 Powered by AgentField

…tatus handler with one batch fetch

handleBatchStatus now issues a single GetExecutionRecordsBatch query
regardless of how many execution IDs are requested, instead of looping
GetExecutionRecord per ID. Missing IDs preserve the prior per-ID
'not_found' response. Requests with more than 500 IDs are rejected with
a 400 before hitting storage. Storage layer gains GetExecutionRecordsBatch
on LocalStorage and the StorageProvider/ExecutionStore interfaces; test
stubs updated to satisfy the new interface.
@AbirAbbas
AbirAbbas requested a review from a team as a code owner July 21, 2026 22:11
@AbirAbbas
AbirAbbas marked this pull request as draft July 21, 2026 22:12
@github-actions

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 86.90% 87.40% ↓ -0.50 pp 🟡
sdk-go 92.40% 92.00% ↑ +0.40 pp 🟢
sdk-python 93.82% 93.73% ↑ +0.09 pp 🟢
sdk-typescript 91.08% 90.42% ↑ +0.66 pp 🟢
web-ui 84.76% 84.79% ↓ -0.03 pp 🟡
aggregate 85.55% 85.75% ↓ -0.20 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 61 90.00%
sdk-go 0 ➖ no changes
sdk-python 0 ➖ no changes
sdk-typescript 0 ➖ no changes
web-ui 0 ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@AbirAbbas
AbirAbbas marked this pull request as ready for review July 22, 2026 13:41
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.

[Control Plane] Batch execution status handler is N+1

1 participant