feat(judge): sampled LLM-as-judge scoring pass (DOS-860) - #9
Conversation
Adds a judge_score table plus a DB-backed scheduler job that samples a configurable fraction of completed agent_task_queue rows, scores them against a rubric (correctness/adherence/tone/clarity) and a separate trajectory score derived from the task_message tool-call sequence, and writes results with rationale, judge model, and the judge's own token/cost usage. Scores are labeled MODELED until a future calibration story lands. Judge is called through an interface so the scheduler job tests run against a fake, deterministic implementation instead of a live Anthropic API key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds an LLM-as-judge pipeline that samples completed tasks, scores trajectories through Anthropic, stores rubric results, exposes them through an API, and displays the latest modeled score in task transcripts. ChangesJudge scoring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant Database
participant TrajectoryBuilder
participant AnthropicJudge
participant AnthropicAPI
participant TranscriptClient
Scheduler->>Database: List unjudged completed tasks
Scheduler->>Database: Insert sampling decision
Scheduler->>TrajectoryBuilder: Build completed-task trajectory
TrajectoryBuilder-->>Scheduler: Return trajectory
Scheduler->>AnthropicJudge: Score trajectory
AnthropicJudge->>AnthropicAPI: Submit rubric tool request
AnthropicAPI-->>AnthropicJudge: Return rubric scores
AnthropicJudge-->>Scheduler: Return judge result
Scheduler->>Database: Upsert judge score
TranscriptClient->>Database: GET task judge scores
Database-->>TranscriptClient: Return score payloads
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ListUnjudgedCompletedTasks previously excluded only scored tasks, so a batch of oldest completed tasks that all missed the sample-rate hash was returned unchanged on every tick forever, permanently halting scoring past that batch. A new judge_sample_decision table records every task the sampler considers (sampled or not), which the candidate query now excludes on instead, guaranteeing forward progress each tick. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
Task descriptions and tool call output are attacker-influenceable content that gets fed straight into the judge's grading prompt. Add a system-turn instruction establishing that content as untrusted data (not commands) and wrap it in <untrusted-task-context>/<untrusted-trajectory> delimiters in the user turn, so an adversarial instruction embedded in a task or tool output can't steer the judge into inflating its own score. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
The integration tests each registered a bare `DELETE FROM judge_score` cleanup, which targets whatever DATABASE_URL points at — the shared local dev DB when unset — deleting every row, not just the test's own fixture rows. seedJudgeFixture's own workspace-delete cleanup already cascades through agent_task_queue to judge_score/judge_sample_decision, so the extra unscoped cleanup was both redundant and unsafe. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
Add GET /api/tasks/:id/judge-scores plus a frontend query + UI section so the DoD's "UI surfaces scores as MODELED" requirement is met: the agent transcript dialog now shows the rubric dimensions, overall score, and rationale for a sampled task, with the calibration_status badge always rendered so users don't mistake MODELED scores for ground truth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
text-amber-600 on bg-amber-500/15 computed to ~2.83:1 in light mode, well under the 4.5:1 AA threshold for normal text. text-amber-800 clears AA with margin; dark mode was already fine. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/views/common/task-transcript/judge-score-section.tsx`:
- Around line 28-50: Update the score state and loading flow in the component
around the useEffect so each response is stored with its originating taskId, and
clear or invalidate the previous result when taskId or taskStatus changes. Guard
rendering of the selected score so it only occurs when the stored task ID
matches the current taskId and taskStatus is "completed"; otherwise return null.
In `@packages/views/locales/ko/agents.json`:
- Around line 461-464: Update the modeled_tooltip value in the judge_score
Korean locale entry to use natural wording equivalent to “모델 추정 점수이며 아직 사람의 검토와
보정을 거치지 않았습니다.” Leave the title and overall translations unchanged.
In `@server/cmd/server/main.go`:
- Around line 384-400: Update the fallback value assigned to judgeModel in the
JUDGE_ANTHROPIC_API_KEY registration block to use Anthropic’s hyphenated model
ID claude-opus-4-8, while preserving the JUDGE_MODEL environment override and
the surrounding scheduler registration flow.
In `@server/internal/judge/anthropic.go`:
- Around line 13-14: Make the Anthropic endpoint configurable through a BaseURL
field on AnthropicJudge, defaulting to anthropicAPIURL when unset. Update the
request-building logic and constructors around AnthropicJudge to use this
resolved base URL, preserving the current production endpoint while allowing
tests and alternate endpoints without custom transport redirection.
In `@server/internal/judge/prompt.go`:
- Around line 43-88: Update BuildPrompt and sanitize every attacker-controlled
value before interpolation, including t.IssueTitle, t.IssueBody, each step’s
Content, Input, and Output, plus t.FinalError and t.FinalResult. Reuse the
existing sanitizeUntrusted helper so literal structural delimiters are
neutralized while preserving the current prompt layout and truncation behavior.
In `@server/internal/scheduler/jobs_judge_score.go`:
- Around line 182-190: Update judgeCallCostUSD so an unrecognized judge model is
not silently treated as a valid zero-cost result: emit an appropriate warning or
error identifying r.Model, while preserving the existing zero return and normal
priced-model calculation.
- Around line 94-161: Update the sampling flow around InsertJudgeSampleDecision,
BuildTrajectory, j.Score, and InsertJudgeScore so sampled=true is recorded only
after the score is successfully persisted. Continue recording sampled=false
immediately for tasks that miss the sample decision, and ensure failures during
trajectory building, scoring, cost conversion, or score insertion leave no
sampled=true marker so the task can be retried on a later tick.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 369bd3de-970f-43ee-be73-f3c3653466a8
⛔ Files ignored due to path filters (2)
server/pkg/db/generated/judge_score.sql.gois excluded by!**/generated/**server/pkg/db/generated/models.gois excluded by!**/generated/**
📒 Files selected for processing (29)
packages/core/api/client.tspackages/core/api/schemas.tspackages/core/types/agent.tspackages/core/types/index.tspackages/views/common/task-transcript/agent-transcript-dialog.tsxpackages/views/common/task-transcript/index.tspackages/views/common/task-transcript/judge-score-section.tsxpackages/views/locales/en/agents.jsonpackages/views/locales/ja/agents.jsonpackages/views/locales/ko/agents.jsonpackages/views/locales/zh-Hans/agents.jsonserver/cmd/server/main.goserver/cmd/server/router.goserver/internal/handler/judge_score.goserver/internal/judge/anthropic.goserver/internal/judge/anthropic_test.goserver/internal/judge/judge.goserver/internal/judge/prompt.goserver/internal/judge/sample.goserver/internal/judge/sample_test.goserver/internal/judge/trajectory.goserver/internal/scheduler/jobs_judge_score.goserver/internal/scheduler/jobs_judge_score_test.goserver/migrations/121_judge_score.down.sqlserver/migrations/121_judge_score.up.sqlserver/migrations/122_judge_sample_decision.down.sqlserver/migrations/122_judge_sample_decision.up.sqlserver/pkg/db/queries/judge_score.sqlserver/pkg/protocol/messages.go
| const [scores, setScores] = useState<JudgeScore[] | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| if (taskStatus !== "completed") return; | ||
| let cancelled = false; | ||
| api | ||
| .listJudgeScores(taskId) | ||
| .then((result) => { | ||
| if (!cancelled) setScores(result); | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled) setScores([]); | ||
| }); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [taskId, taskStatus]); | ||
|
|
||
| if (!scores || scores.length === 0) return null; | ||
|
|
||
| // A task can be scored by more than one judge model over time; show the | ||
| // most recent pass (GetJudgeScoresByTask orders newest-first). | ||
| const score = scores[0]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope loaded scores to the current task.
loadedScores is never cleared or associated with taskId. If the dialog receives another task, the previous score remains visible until the new request finishes; if the new task is not completed, the early return leaves the wrong score rendered indefinitely. Store the task ID with the response and guard rendering against both taskId and taskStatus.
Proposed fix
- const [scores, setScores] = useState<JudgeScore[] | null>(null);
+ const [loadedScores, setLoadedScores] = useState<{
+ taskId: string;
+ scores: JudgeScore[];
+ } | null>(null);
...
- if (!cancelled) setScores(result);
+ if (!cancelled) setLoadedScores({ taskId, scores: result });
...
- if (!scores || scores.length === 0) return null;
+ if (
+ taskStatus !== "completed" ||
+ !loadedScores ||
+ loadedScores.taskId !== taskId ||
+ loadedScores.scores.length === 0
+ ) {
+ return null;
+ }
- const score = scores[0];
+ const score = loadedScores.scores[0];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [scores, setScores] = useState<JudgeScore[] | null>(null); | |
| useEffect(() => { | |
| if (taskStatus !== "completed") return; | |
| let cancelled = false; | |
| api | |
| .listJudgeScores(taskId) | |
| .then((result) => { | |
| if (!cancelled) setScores(result); | |
| }) | |
| .catch(() => { | |
| if (!cancelled) setScores([]); | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [taskId, taskStatus]); | |
| if (!scores || scores.length === 0) return null; | |
| // A task can be scored by more than one judge model over time; show the | |
| // most recent pass (GetJudgeScoresByTask orders newest-first). | |
| const score = scores[0]; | |
| const [loadedScores, setLoadedScores] = useState<{ | |
| taskId: string; | |
| scores: JudgeScore[]; | |
| } | null>(null); | |
| useEffect(() => { | |
| if (taskStatus !== "completed") return; | |
| let cancelled = false; | |
| api | |
| .listJudgeScores(taskId) | |
| .then((result) => { | |
| if (!cancelled) setLoadedScores({ taskId, scores: result }); | |
| }) | |
| .catch(() => { | |
| if (!cancelled) setLoadedScores(null); | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [taskId, taskStatus]); | |
| if ( | |
| taskStatus !== "completed" || | |
| !loadedScores || | |
| loadedScores.taskId !== taskId || | |
| loadedScores.scores.length === 0 | |
| ) { | |
| return null; | |
| } | |
| // A task can be scored by more than one judge model over time; show the | |
| // most recent pass (GetJudgeScoresByTask orders newest-first). | |
| const score = loadedScores.scores[0]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/views/common/task-transcript/judge-score-section.tsx` around lines
28 - 50, Update the score state and loading flow in the component around the
useEffect so each response is stored with its originating taskId, and clear or
invalidate the previous result when taskId or taskStatus changes. Guard
rendering of the selected score so it only occurs when the stored task ID
matches the current taskId and taskStatus is "completed"; otherwise return null.
| "judge_score": { | ||
| "title": "심사 점수", | ||
| "modeled_tooltip": "모델링된 점수이며 아직 사람 검토와 보정되지 않았습니다", | ||
| "overall": "전체 {{score}}/100", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Korean modeled-score tooltip wording.
Use a natural phrasing such as 모델 추정 점수이며 아직 사람의 검토와 보정을 거치지 않았습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/views/locales/ko/agents.json` around lines 461 - 464, Update the
modeled_tooltip value in the judge_score Korean locale entry to use natural
wording equivalent to “모델 추정 점수이며 아직 사람의 검토와 보정을 거치지 않았습니다.” Leave the title and
overall translations unchanged.
| // DOS-860: sampled LLM-as-judge scoring pass. Registered only when | ||
| // an Anthropic API key is configured — with no key there is no | ||
| // judge to call, so the job would just fail every attempt. | ||
| if apiKey := os.Getenv("JUDGE_ANTHROPIC_API_KEY"); apiKey != "" { | ||
| judgeModel := os.Getenv("JUDGE_MODEL") | ||
| if judgeModel == "" { | ||
| judgeModel = "claude-opus-4.8" | ||
| } | ||
| judgeCfg := scheduler.DefaultJudgeScoreSamplerConfig() | ||
| if rate, ok := envFloatInUnitInterval("JUDGE_SAMPLE_RATE"); ok { | ||
| judgeCfg.SampleRate = rate | ||
| } | ||
| anthropicJudge := judge.NewAnthropicJudge(apiKey, judgeModel, nil) | ||
| if err := schedulerMgr.Register(scheduler.JudgeScoreSamplerJob(pool, anthropicJudge, judgeCfg)); err != nil { | ||
| slog.Warn("scheduler: failed to register judge_score_sampler job", "error", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What is the correct Anthropic API model ID for Claude Opus 4.8 — claude-opus-4-8 or claude-opus-4.8?
💡 Result:
The correct Anthropic API model ID for Claude Opus 4.8 is claude-opus-4-8 [1][2][3]. Official documentation and resources from Anthropic consistently use this identifier [2][4][5]. It does not contain a period (dot) in the string, as evidenced by its structure following the standard claude-{name}-{major}[-{minor}] format used for the Claude 4.6 generation and later [6].
Citations:
- 1: https://www.anthropic.com/claude/opus
- 2: https://platform.claude.com/docs/en/about-claude/models/whats-new-claude-4-8
- 3: https://platform.claude.com/docs/en/release-notes/overview
- 4: https://platform.claude.com/docs/en/about-claude/models/overview?fcdaa149_sort_date=desc&us=
- 5: https://platform.claude.com/docs/en/about-claude/models/overview
- 6: https://platform.claude.com/docs/en/about-claude/models/model-ids-and-versions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the judge job and cost lookup paths referenced by the comment.
git ls-files 'server/cmd/server/main.go' 'server/jobs_judge_score.go' 'server/**/judge*' | sed 's/^/FILE /'
printf '\n--- server/jobs_judge_score.go (relevant sections) ---\n'
sed -n '1,260p' server/jobs_judge_score.go
printf '\n--- search for PriceForModelAlias usage ---\n'
rg -n 'PriceForModelAlias|judgeCallCostUSD|ModelAlias|result\.Model' server -SRepository: firefly-events/multica
Length of output: 730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- server/internal/handler/judge_score.go ---\n'
wc -l server/internal/handler/judge_score.go
sed -n '1,260p' server/internal/handler/judge_score.go
printf '\n--- server/internal/judge/anthropic.go ---\n'
wc -l server/internal/judge/anthropic.go
sed -n '1,260p' server/internal/judge/anthropic.go
printf '\n--- search for cost lookup / pricing aliases in server/internal ---\n'
rg -n 'PriceForModelAlias|model alias|ModelAlias|judgeCallCostUSD|costUSD|PriceForModel|price table|PriceFor' server/internal server/pkg -SRepository: firefly-events/multica
Length of output: 11417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- server/internal/scheduler/jobs_judge_score.go ---\n'
wc -l server/internal/scheduler/jobs_judge_score.go
sed -n '160,240p' server/internal/scheduler/jobs_judge_score.go
printf '\n--- server/internal/metrics/pricing.go ---\n'
wc -l server/internal/metrics/pricing.go
sed -n '1,220p' server/internal/metrics/pricing.go
printf '\n--- server/internal/metrics/pricing_test.go ---\n'
wc -l server/internal/metrics/pricing_test.go
sed -n '1,120p' server/internal/metrics/pricing_test.goRepository: firefly-events/multica
Length of output: 8064
Default judge model ID should use hyphens
JUDGE_MODEL falls back to claude-opus-4.8, but Anthropic uses claude-opus-4-8. When the env var is unset, the judge call is sent with an invalid model ID and scoring fails.
🔧 Proposed fix
- judgeModel = "claude-opus-4.8"
+ judgeModel = "claude-opus-4-8"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DOS-860: sampled LLM-as-judge scoring pass. Registered only when | |
| // an Anthropic API key is configured — with no key there is no | |
| // judge to call, so the job would just fail every attempt. | |
| if apiKey := os.Getenv("JUDGE_ANTHROPIC_API_KEY"); apiKey != "" { | |
| judgeModel := os.Getenv("JUDGE_MODEL") | |
| if judgeModel == "" { | |
| judgeModel = "claude-opus-4.8" | |
| } | |
| judgeCfg := scheduler.DefaultJudgeScoreSamplerConfig() | |
| if rate, ok := envFloatInUnitInterval("JUDGE_SAMPLE_RATE"); ok { | |
| judgeCfg.SampleRate = rate | |
| } | |
| anthropicJudge := judge.NewAnthropicJudge(apiKey, judgeModel, nil) | |
| if err := schedulerMgr.Register(scheduler.JudgeScoreSamplerJob(pool, anthropicJudge, judgeCfg)); err != nil { | |
| slog.Warn("scheduler: failed to register judge_score_sampler job", "error", err) | |
| } | |
| } | |
| // DOS-860: sampled LLM-as-judge scoring pass. Registered only when | |
| // an Anthropic API key is configured — with no key there is no | |
| // judge to call, so the job would just fail every attempt. | |
| if apiKey := os.Getenv("JUDGE_ANTHROPIC_API_KEY"); apiKey != "" { | |
| judgeModel := os.Getenv("JUDGE_MODEL") | |
| if judgeModel == "" { | |
| judgeModel = "claude-opus-4-8" | |
| } | |
| judgeCfg := scheduler.DefaultJudgeScoreSamplerConfig() | |
| if rate, ok := envFloatInUnitInterval("JUDGE_SAMPLE_RATE"); ok { | |
| judgeCfg.SampleRate = rate | |
| } | |
| anthropicJudge := judge.NewAnthropicJudge(apiKey, judgeModel, nil) | |
| if err := schedulerMgr.Register(scheduler.JudgeScoreSamplerJob(pool, anthropicJudge, judgeCfg)); err != nil { | |
| slog.Warn("scheduler: failed to register judge_score_sampler job", "error", err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/cmd/server/main.go` around lines 384 - 400, Update the fallback value
assigned to judgeModel in the JUDGE_ANTHROPIC_API_KEY registration block to use
Anthropic’s hyphenated model ID claude-opus-4-8, while preserving the
JUDGE_MODEL environment override and the surrounding scheduler registration
flow.
| const anthropicAPIURL = "https://api.anthropic.com/v1/messages" | ||
| const anthropicAPIVersion = "2023-06-01" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider making the API base URL configurable instead of a fixed constant.
anthropicAPIURL is hardcoded, forcing anthropic_test.go to build a custom RoundTripper just to redirect requests to a fake server. A BaseURL field on AnthropicJudge (defaulting to anthropicAPIURL when empty) would simplify tests and support alternate/self-hosted endpoints without behavior change for production callers.
Also applies to: 43-56, 137-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/internal/judge/anthropic.go` around lines 13 - 14, Make the Anthropic
endpoint configurable through a BaseURL field on AnthropicJudge, defaulting to
anthropicAPIURL when unset. Update the request-building logic and constructors
around AnthropicJudge to use this resolved base URL, preserving the current
production endpoint while allowing tests and alternate endpoints without custom
transport redirection.
| func BuildPrompt(t Trajectory) string { | ||
| var b strings.Builder | ||
| b.WriteString(rubricInstructions) | ||
|
|
||
| b.WriteString("\n\n<untrusted-task-context>\n") | ||
| fmt.Fprintf(&b, "Title: %s\n\n", t.IssueTitle) | ||
| if t.IssueBody != "" { | ||
| fmt.Fprintf(&b, "Description:\n%s\n\n", t.IssueBody) | ||
| } | ||
| b.WriteString("</untrusted-task-context>\n") | ||
|
|
||
| b.WriteString("\n<untrusted-trajectory>\n") | ||
| if len(t.Steps) == 0 { | ||
| b.WriteString("(no recorded tool-call steps)\n\n") | ||
| } | ||
| for _, s := range t.Steps { | ||
| fmt.Fprintf(&b, "%d. [%s]", s.Seq, s.Type) | ||
| if s.Tool != "" { | ||
| fmt.Fprintf(&b, " tool=%s", s.Tool) | ||
| } | ||
| b.WriteString("\n") | ||
| if s.Content != "" { | ||
| fmt.Fprintf(&b, " content: %s\n", truncate(s.Content, 2000)) | ||
| } | ||
| if s.Input != "" { | ||
| fmt.Fprintf(&b, " input: %s\n", truncate(s.Input, 2000)) | ||
| } | ||
| if s.Output != "" { | ||
| fmt.Fprintf(&b, " output: %s\n", truncate(s.Output, 2000)) | ||
| } | ||
| } | ||
|
|
||
| b.WriteString("\n## Final outcome\n\n") | ||
| if t.FinalError != "" { | ||
| fmt.Fprintf(&b, "error: %s\n", truncate(t.FinalError, 4000)) | ||
| } | ||
| if t.FinalResult != "" { | ||
| fmt.Fprintf(&b, "result: %s\n", truncate(t.FinalResult, 8000)) | ||
| } | ||
| if t.FinalError == "" && t.FinalResult == "" { | ||
| b.WriteString("(no recorded result or error)\n") | ||
| } | ||
| b.WriteString("</untrusted-trajectory>\n") | ||
|
|
||
| return b.String() | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Untrusted content can forge the tag delimiters to escape the sandbox.
t.IssueTitle, t.IssueBody, and each step's Content/Input/Output are interpolated verbatim between the <untrusted-task-context>/<untrusted-trajectory> tags. Since these tags are the only structural boundary the model relies on (per judgeSystemPrompt), any attacker-controlled content containing the literal closing tag (e.g. </untrusted-trajectory>) can prematurely close the block, inject fake grading instructions, and re-open the tag to keep the prompt well-formed. This defeats the injection protection this PR is meant to add — task descriptions, tool output, and web/file content the agent touched are all attacker-influenceable per the file's own doc comment.
Neutralize the delimiter strings in untrusted content before interpolation (or use a per-request random tag suffix that can't be guessed/forged) rather than relying solely on the system-prompt instruction.
🔒 Proposed fix
+// sanitizeUntrusted strips any occurrence of the judge's own delimiter
+// tags from untrusted content so it cannot forge a tag boundary and
+// escape the <untrusted-*> block.
+func sanitizeUntrusted(s string) string {
+ for _, tag := range []string{
+ "<untrusted-task-context>", "</untrusted-task-context>",
+ "<untrusted-trajectory>", "</untrusted-trajectory>",
+ } {
+ s = strings.ReplaceAll(s, tag, "")
+ }
+ return s
+}
+
func BuildPrompt(t Trajectory) string {
var b strings.Builder
b.WriteString(rubricInstructions)
b.WriteString("\n\n<untrusted-task-context>\n")
- fmt.Fprintf(&b, "Title: %s\n\n", t.IssueTitle)
- if t.IssueBody != "" {
- fmt.Fprintf(&b, "Description:\n%s\n\n", t.IssueBody)
+ fmt.Fprintf(&b, "Title: %s\n\n", sanitizeUntrusted(t.IssueTitle))
+ if t.IssueBody != "" {
+ fmt.Fprintf(&b, "Description:\n%s\n\n", sanitizeUntrusted(t.IssueBody))
}Apply the same sanitizeUntrusted wrap to s.Content, s.Input, s.Output, t.FinalResult, and t.FinalError below.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/internal/judge/prompt.go` around lines 43 - 88, Update BuildPrompt and
sanitize every attacker-controlled value before interpolation, including
t.IssueTitle, t.IssueBody, each step’s Content, Input, and Output, plus
t.FinalError and t.FinalResult. Reuse the existing sanitizeUntrusted helper so
literal structural delimiters are neutralized while preserving the current
prompt layout and truncation behavior.
| for _, task := range candidates { | ||
| taskID := task.ID.String() | ||
| decision := judge.ShouldSample(taskID, cfg.SampleRate) | ||
|
|
||
| // Record that this task was considered regardless of the | ||
| // sample outcome. ListUnjudgedCompletedTasks excludes on | ||
| // judge_sample_decision, not judge_score, so this is what | ||
| // lets the candidate window advance past a task that missed | ||
| // the sample-rate hash instead of returning it forever. | ||
| if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{ | ||
| TaskID: task.ID, | ||
| Sampled: decision, | ||
| }); err != nil { | ||
| failed++ | ||
| continue | ||
| } | ||
|
|
||
| if !decision { | ||
| continue | ||
| } | ||
| sampled++ | ||
|
|
||
| if in.Heartbeat != nil { | ||
| _ = in.Heartbeat(ctx) | ||
| } | ||
|
|
||
| trajectory, err := judge.BuildTrajectory(ctx, q, task) | ||
| if err != nil { | ||
| failed++ | ||
| continue | ||
| } | ||
|
|
||
| result, err := j.Score(ctx, judge.Input{Trajectory: trajectory}) | ||
| if err != nil { | ||
| failed++ | ||
| continue | ||
| } | ||
|
|
||
| costUSD := judgeCallCostUSD(result) | ||
|
|
||
| var costNumeric pgtype.Numeric | ||
| if err := costNumeric.Scan(fmt.Sprintf("%.6f", costUSD)); err != nil { | ||
| failed++ | ||
| continue | ||
| } | ||
|
|
||
| if _, err := q.InsertJudgeScore(ctx, db.InsertJudgeScoreParams{ | ||
| TaskID: task.ID, | ||
| JudgeProvider: result.Provider, | ||
| JudgeModel: result.Model, | ||
| CorrectnessScore: int32(result.CorrectnessScore), | ||
| AdherenceScore: int32(result.AdherenceScore), | ||
| ToneScore: int32(result.ToneScore), | ||
| ClarityScore: int32(result.ClarityScore), | ||
| TrajectoryScore: int32(result.TrajectoryScore), | ||
| OverallScore: int32(result.OverallScore), | ||
| Rationale: result.Rationale, | ||
| TrajectoryRationale: result.TrajectoryRationale, | ||
| CalibrationStatus: judge.ModeledStatus, | ||
| InputTokens: result.InputTokens, | ||
| OutputTokens: result.OutputTokens, | ||
| CostUsd: costNumeric, | ||
| }); err != nil { | ||
| failed++ | ||
| continue | ||
| } | ||
| scored++ | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Sampled tasks that fail to score are silently lost forever.
InsertJudgeSampleDecision(Sampled: true) is written before BuildTrajectory/j.Score/InsertJudgeScore run. If any of those calls fail, the task is failed++ and skipped, but the sample-decision row still exists — so ListUnjudgedCompletedTasks' NOT EXISTS (... judge_sample_decision ...) clause permanently excludes it from every future tick. A transient judge API error (rate limit, timeout) forever burns that task's only chance to be judged, and the job's own MaxAttempts/RetryBackoff never engages since per-task errors don't bubble up as a handler error.
Defer recording sampled=true until after the score is actually persisted; record sampled=false immediately since that path never needs a retry.
🔧 Proposed fix
for _, task := range candidates {
taskID := task.ID.String()
decision := judge.ShouldSample(taskID, cfg.SampleRate)
- // Record that this task was considered regardless of the
- // sample outcome. ListUnjudgedCompletedTasks excludes on
- // judge_sample_decision, not judge_score, so this is what
- // lets the candidate window advance past a task that missed
- // the sample-rate hash instead of returning it forever.
- if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{
- TaskID: task.ID,
- Sampled: decision,
- }); err != nil {
- failed++
- continue
- }
-
if !decision {
+ // Record the "not sampled" decision immediately so the
+ // candidate window advances past this task; no retry is
+ // ever needed for a task that was never scored.
+ if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{
+ TaskID: task.ID,
+ Sampled: false,
+ }); err != nil {
+ failed++
+ }
continue
}
sampled++
if in.Heartbeat != nil {
_ = in.Heartbeat(ctx)
}
trajectory, err := judge.BuildTrajectory(ctx, q, task)
if err != nil {
failed++
- continue
+ continue // decision not recorded — retried next tick
}
result, err := j.Score(ctx, judge.Input{Trajectory: trajectory})
if err != nil {
failed++
- continue
+ continue // decision not recorded — retried next tick
}
costUSD := judgeCallCostUSD(result)
var costNumeric pgtype.Numeric
if err := costNumeric.Scan(fmt.Sprintf("%.6f", costUSD)); err != nil {
failed++
continue
}
if _, err := q.InsertJudgeScore(ctx, db.InsertJudgeScoreParams{
TaskID: task.ID,
JudgeProvider: result.Provider,
JudgeModel: result.Model,
CorrectnessScore: int32(result.CorrectnessScore),
AdherenceScore: int32(result.AdherenceScore),
ToneScore: int32(result.ToneScore),
ClarityScore: int32(result.ClarityScore),
TrajectoryScore: int32(result.TrajectoryScore),
OverallScore: int32(result.OverallScore),
Rationale: result.Rationale,
TrajectoryRationale: result.TrajectoryRationale,
CalibrationStatus: judge.ModeledStatus,
InputTokens: result.InputTokens,
OutputTokens: result.OutputTokens,
CostUsd: costNumeric,
}); err != nil {
failed++
continue
}
+
+ if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{
+ TaskID: task.ID,
+ Sampled: true,
+ }); err != nil {
+ failed++
+ }
scored++
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _, task := range candidates { | |
| taskID := task.ID.String() | |
| decision := judge.ShouldSample(taskID, cfg.SampleRate) | |
| // Record that this task was considered regardless of the | |
| // sample outcome. ListUnjudgedCompletedTasks excludes on | |
| // judge_sample_decision, not judge_score, so this is what | |
| // lets the candidate window advance past a task that missed | |
| // the sample-rate hash instead of returning it forever. | |
| if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{ | |
| TaskID: task.ID, | |
| Sampled: decision, | |
| }); err != nil { | |
| failed++ | |
| continue | |
| } | |
| if !decision { | |
| continue | |
| } | |
| sampled++ | |
| if in.Heartbeat != nil { | |
| _ = in.Heartbeat(ctx) | |
| } | |
| trajectory, err := judge.BuildTrajectory(ctx, q, task) | |
| if err != nil { | |
| failed++ | |
| continue | |
| } | |
| result, err := j.Score(ctx, judge.Input{Trajectory: trajectory}) | |
| if err != nil { | |
| failed++ | |
| continue | |
| } | |
| costUSD := judgeCallCostUSD(result) | |
| var costNumeric pgtype.Numeric | |
| if err := costNumeric.Scan(fmt.Sprintf("%.6f", costUSD)); err != nil { | |
| failed++ | |
| continue | |
| } | |
| if _, err := q.InsertJudgeScore(ctx, db.InsertJudgeScoreParams{ | |
| TaskID: task.ID, | |
| JudgeProvider: result.Provider, | |
| JudgeModel: result.Model, | |
| CorrectnessScore: int32(result.CorrectnessScore), | |
| AdherenceScore: int32(result.AdherenceScore), | |
| ToneScore: int32(result.ToneScore), | |
| ClarityScore: int32(result.ClarityScore), | |
| TrajectoryScore: int32(result.TrajectoryScore), | |
| OverallScore: int32(result.OverallScore), | |
| Rationale: result.Rationale, | |
| TrajectoryRationale: result.TrajectoryRationale, | |
| CalibrationStatus: judge.ModeledStatus, | |
| InputTokens: result.InputTokens, | |
| OutputTokens: result.OutputTokens, | |
| CostUsd: costNumeric, | |
| }); err != nil { | |
| failed++ | |
| continue | |
| } | |
| scored++ | |
| } | |
| for _, task := range candidates { | |
| taskID := task.ID.String() | |
| decision := judge.ShouldSample(taskID, cfg.SampleRate) | |
| if !decision { | |
| // Record the "not sampled" decision immediately so the | |
| // candidate window advances past this task; no retry is | |
| // ever needed for a task that was never scored. | |
| if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{ | |
| TaskID: task.ID, | |
| Sampled: false, | |
| }); err != nil { | |
| failed++ | |
| } | |
| continue | |
| } | |
| sampled++ | |
| if in.Heartbeat != nil { | |
| _ = in.Heartbeat(ctx) | |
| } | |
| trajectory, err := judge.BuildTrajectory(ctx, q, task) | |
| if err != nil { | |
| failed++ | |
| continue // decision not recorded — retried next tick | |
| } | |
| result, err := j.Score(ctx, judge.Input{Trajectory: trajectory}) | |
| if err != nil { | |
| failed++ | |
| continue // decision not recorded — retried next tick | |
| } | |
| costUSD := judgeCallCostUSD(result) | |
| var costNumeric pgtype.Numeric | |
| if err := costNumeric.Scan(fmt.Sprintf("%.6f", costUSD)); err != nil { | |
| failed++ | |
| continue | |
| } | |
| if _, err := q.InsertJudgeScore(ctx, db.InsertJudgeScoreParams{ | |
| TaskID: task.ID, | |
| JudgeProvider: result.Provider, | |
| JudgeModel: result.Model, | |
| CorrectnessScore: int32(result.CorrectnessScore), | |
| AdherenceScore: int32(result.AdherenceScore), | |
| ToneScore: int32(result.ToneScore), | |
| ClarityScore: int32(result.ClarityScore), | |
| TrajectoryScore: int32(result.TrajectoryScore), | |
| OverallScore: int32(result.OverallScore), | |
| Rationale: result.Rationale, | |
| TrajectoryRationale: result.TrajectoryRationale, | |
| CalibrationStatus: judge.ModeledStatus, | |
| InputTokens: result.InputTokens, | |
| OutputTokens: result.OutputTokens, | |
| CostUsd: costNumeric, | |
| }); err != nil { | |
| failed++ | |
| continue | |
| } | |
| if err := q.InsertJudgeSampleDecision(ctx, db.InsertJudgeSampleDecisionParams{ | |
| TaskID: task.ID, | |
| Sampled: true, | |
| }); err != nil { | |
| failed++ | |
| } | |
| scored++ | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 143-143: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(result.CorrectnessScore)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 144-144: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(result.AdherenceScore)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 145-145: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(result.ToneScore)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 146-146: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(result.ClarityScore)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 147-147: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(result.TrajectoryScore)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 148-148: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(result.OverallScore)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/internal/scheduler/jobs_judge_score.go` around lines 94 - 161, Update
the sampling flow around InsertJudgeSampleDecision, BuildTrajectory, j.Score,
and InsertJudgeScore so sampled=true is recorded only after the score is
successfully persisted. Continue recording sampled=false immediately for tasks
that miss the sample decision, and ensure failures during trajectory building,
scoring, cost conversion, or score insertion leave no sampled=true marker so the
task can be retried on a later tick.
| func judgeCallCostUSD(r judge.Result) float64 { | ||
| price, ok := metrics.PriceForModelAlias(r.Model) | ||
| if !ok { | ||
| return 0 | ||
| } | ||
| input := float64(r.InputTokens) * price.InputPerM / 1_000_000 | ||
| output := float64(r.OutputTokens) * price.OutputPerM / 1_000_000 | ||
| return input + output | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Silent zero-cost on unpriced judge model.
If metrics.PriceForModelAlias(r.Model) doesn't recognize the model, cost_usd is silently persisted as 0 with no warning — undermining the "judge's own cost is logged/bounded" goal without any signal that pricing is missing.
🔧 Suggested fix
func judgeCallCostUSD(r judge.Result) float64 {
price, ok := metrics.PriceForModelAlias(r.Model)
if !ok {
+ slog.Warn("judge: no price table entry for model; cost_usd recorded as 0", "model", r.Model)
return 0
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func judgeCallCostUSD(r judge.Result) float64 { | |
| price, ok := metrics.PriceForModelAlias(r.Model) | |
| if !ok { | |
| return 0 | |
| } | |
| input := float64(r.InputTokens) * price.InputPerM / 1_000_000 | |
| output := float64(r.OutputTokens) * price.OutputPerM / 1_000_000 | |
| return input + output | |
| } | |
| func judgeCallCostUSD(r judge.Result) float64 { | |
| price, ok := metrics.PriceForModelAlias(r.Model) | |
| if !ok { | |
| slog.Warn("judge: no price table entry for model; cost_usd recorded as 0", "model", r.Model) | |
| return 0 | |
| } | |
| input := float64(r.InputTokens) * price.InputPerM / 1_000_000 | |
| output := float64(r.OutputTokens) * price.OutputPerM / 1_000_000 | |
| return input + output | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/internal/scheduler/jobs_judge_score.go` around lines 182 - 190, Update
judgeCallCostUSD so an unrecognized judge model is not silently treated as a
valid zero-cost result: emit an appropriate warning or error identifying
r.Model, while preserving the existing zero return and normal priced-model
calculation.
Locks down the skip-when-not-completed, empty-result, API-rejection, unmount-cancellation, and happy-path render behaviors of the LLM-as-judge score section (DOS-1472). Co-authored-by: multica-agent <github@multica.ai>
1 participant
Summary
judge_scoretable (migration 121) linked toagent_task_queue, storing correctness/adherence/tone/clarity/trajectory/overall scores (0-100), rationale, judge provider/model, token usage, cost, andcalibration_status(defaults toMODELEDuntil a future calibration story lands).internal/judgepackage: rubric prompt builder, trajectory builder (readstask_messageordered byseq+ issue context so the judge grades the tool-call sequence, not just the final diff), and an Anthropic Messages API-backedJudgeimplementation (tool-forced JSON output).judge_score_samplerjob registered on the existing DB-backed execution scheduler (internal/scheduler, MUL-2957) rather than a plain ticker — gives leasing/retry/audit for free and matches "no box cron". Sampling is a deterministic hash of task id vs a configured rate (envJUDGE_SAMPLE_RATE, default 5%), so retries don't re-roll the coin. Only registered whenJUDGE_ANTHROPIC_API_KEYis set.internal/metricsprice table and stored per score row, so the judge's cost is observable and bounded (BatchLimitcaps candidates per tick).Test plan
go build ./...,go vet ./...go test ./internal/judge/...— sample-rate boundary/determinism/convergence, Anthropic tool-use response parsing against a fake HTTP servergo test ./internal/scheduler/...— integration tests against real Postgres with a fakeJudge: sampled subset gets scored (not all), 0% rate scores nothing, already-scored tasks aren't re-billed on a second run🤖 Generated with Claude Code
Summary by CodeRabbit