feat: publish batching and scoring request logs#298
Closed
albertywu wants to merge 1 commit into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends SubmitQueue’s request-log stream to make pipeline progress more visible by introducing a new scoring request status and emitting batching/scoring log entries earlier in the orchestrator pipeline.
Changes:
- Add
RequestStatusScoringto the request log status enum for gateway-visible score progress. - Publish
batchingrequest logs inmergeconflictsignalbefore handing mergeable requests to the batch topic. - Publish
scoringrequest logs inscorebefore invoking the scorer, while keeping existingscoredlogs after persistence.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| submitqueue/orchestrator/controller/score/score.go | Publishes per-request scoring logs prior to scoring a batch. |
| submitqueue/orchestrator/controller/score/score_test.go | Adjusts publisher mock behavior to ignore log-topic publishes during score tests. |
| submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal.go | Publishes a batching log entry before publishing the request ID to the batch topic. |
| submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal_test.go | Updates test topic registry and publisher mock to tolerate the new log publish. |
| submitqueue/entity/request_log.go | Adds the new scoring request status constant. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
05a6d75 to
145244c
Compare
145244c to
83c5c30
Compare
83c5c30 to
6ef5943
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
submitqueue/orchestrator/controller/score/score_test.go:55
requestLogsFromMessagesis introduced but never used in this test file, which adds dead code and can confuse future readers. Either remove it or add assertions that use it (e.g. validating the new scoring/scored request-log publishes).
// testBatch returns a standard test batch for score tests.
func testBatch() entity.Batch {
return entity.Batch{
ID: "test-queue/batch/1",
Queue: "test-queue",
Contains: []string{"test-queue/1"},
State: entity.BatchStateCreated,
Version: 1,
}
}
Comment on lines
+121
to
+125
| batchingLog := entity.NewRequestLog(request.ID, entity.RequestStatusBatching, 0, "", nil) | ||
| if err := corerequest.PublishLog(ctx, c.registry, batchingLog, request.ID); err != nil { | ||
| metrics.NamedCounter(c.metricsScope, opName, "request_log_errors", 1) | ||
| return fmt.Errorf("failed to publish batching request log for request %s: %w", request.ID, err) | ||
| } |
Emit gateway-visible progress logs when requests enter active batching and scoring, while keeping tests focused on existing controller behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
6ef5943 to
5a36a03
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scoringrequest log statusscoringandbatchingrequest log status upon controller start. There is already ascoredandbatchedrequest log that is published upon controller completion.Test plan
✅
make fmt && make build && make test && make check-mocks && make e2e-test