[fix] Show evaluators in annotation queues - #6263
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe evaluation-run mapping schema now accepts arbitrary string kinds while retaining known kind literals for TypeScript narrowing. Unit tests cover ChangesEvaluation-run mapping compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to The PR fixes evaluator visibility by accepting backend mapping kinds, but the current head still has a lint-blocking unused declaration and a test import that bypasses the public package entry point. Merge should wait for these bounded cleanup fixes. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8cf45977-efce-4bff-9ef9-6413b9a96fa7
📒 Files selected for processing (2)
web/packages/agenta-entities/src/evaluationRun/core/schema.tsweb/packages/agenta-entities/tests/unit/evaluationRunsResponseSchema.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| const EVALUATION_RUN_MAPPING_KINDS = [ | ||
| "testset", | ||
| "query", | ||
| "invocation", | ||
| "annotation", | ||
| // legacy / alternate taxonomy still accepted defensively | ||
| "input", | ||
| "ground_truth", | ||
| "application", | ||
| "evaluator", | ||
| "annotation", | ||
| ]) | ||
| export type EvaluationRunMappingKind = z.infer<typeof evaluationRunMappingKindSchema> | ||
| ] as const | ||
| // The backend defines mapping kinds as a free-form string and may add values. | ||
| export const evaluationRunMappingKindSchema = z.string() | ||
| export type EvaluationRunMappingKind = (typeof EVALUATION_RUN_MAPPING_KINDS)[number] | (string & {}) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="web/packages/agenta-entities/src/evaluationRun/core/schema.ts"
printf '%s\n' '--- schema excerpt ---'
sed -n '1,80p' "$file"
printf '%s\n' '--- references to the declaration ---'
rg -n --fixed-strings 'EVALUATION_RUN_MAPPING_KINDS' web/packages/agenta-entities web 2>/dev/null || true
printf '%s\n' '--- lint configuration and package scripts ---'
find web -maxdepth 3 \( -name 'package.json' -o -name '*eslint*' -o -name 'tsconfig*.json' \) -print
rg -n 'no-unused-vars|noUnusedLocals|lint' web/packages/agenta-entities web/package.json web/*eslint* web/*tsconfig* 2>/dev/null || trueRepository: Agenta-AI/agenta
Length of output: 10967
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- unused-variable configuration ---'
sed -n '88,112p' web/eslint.config.mjs
printf '%s\n' '--- entity package configuration ---'
cat web/packages/agenta-entities/package.json
printf '%s\n' '--- TypeScript compiler options ---'
cat web/packages/agenta-entities/tsconfig.json
sed -n '1,120p' web/packages/tsconfig.base.jsonRepository: Agenta-AI/agenta
Length of output: 9515
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- available local tooling ---'
command -v node || true
command -v pnpm || true
test -x web/node_modules/.bin/eslint && echo 'eslint binary present' || echo 'eslint binary absent'
test -x web/node_modules/.bin/tsc && echo 'tsc binary present' || echo 'tsc binary absent'
printf '%s\n' '--- pinned tool versions ---'
rg -n '"`@typescript-eslint/`(eslint-plugin|parser)"|typescript@|eslint@' web/pnpm-lock.yaml pnpm-lock.yaml 2>/dev/null | head -30 || trueRepository: Agenta-AI/agenta
Length of output: 3044
🌐 Web query:
@typescript-eslint/no-unused-vars variable used in typeof type query considered used
💡 Result:
The @typescript-eslint/no-unused-vars rule does not consider variables used if they are only referenced in a typeof type query [1][2]. In previous versions, the rule may have permitted such usage, but as of the v8 release, @typescript-eslint/no-unused-vars was updated to report variables that are used only in type positions [3][4]. The maintainers intentionally designed this behavior because declaring runtime variables solely for use in types adds unnecessary code and complexity, as these variables do not exist at runtime [1][5]. If you encounter this linting error, the recommended approach is to refactor the code to define the necessary types directly rather than deriving them from a runtime variable [1][2]. For example, instead of: const box = { value: 123 }; export type Box = typeof box; You should use: export interface Box { value: number; } This practice avoids redundant runtime code and makes the intent clearer [1][4]. While there have been requests to add an option to allow variables to be used only as types, the maintainers have declined these, opting instead to encourage cleaner type-only declarations [4].
Citations:
- 1: https://typescript-eslint.io/rules/no-unused-vars
- 2: https://typescript-eslint.io/rules/no-unused-vars/
- 3: feat(eslint-plugin): [no-unused-vars] report if var used only in typeof typescript-eslint/typescript-eslint#9330
- 4: Enhancement: [no-unused-vars] Add option to allow variables to be used only as types or allow 'declare' typescript-eslint/typescript-eslint#10266
- 5: typescript-eslint/typescript-eslint@974f2b1
Remove the unused runtime declaration.
@typescript-eslint/no-unused-vars reports variables referenced only through typeof. Replace EVALUATION_RUN_MAPPING_KINDS with a type-only literal union, or export it when runtime consumers need the taxonomy.
🧰 Tools
🪛 GitHub Check: TypeScript lint
[failure] 28-28:
'EVALUATION_RUN_MAPPING_KINDS' is assigned a value but only used as a type. Allowed unused vars must match /^|^.*/u
Source: Linters/SAST tools
| @@ -0,0 +1,72 @@ | |||
| import {describe, expect, it} from "vitest" | |||
|
|
|||
| import {evaluationRunsResponseSchema} from "../../src/evaluationRun/core/schema" | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a '^package\.json$' web/packages/agenta-entities \
-x rg -n -C2 '"exports"|evaluationRun' {}Repository: Agenta-AI/agenta
Length of output: 1304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package exports ---'
sed -n '28,52p' web/packages/agenta-entities/package.json
printf '%s\n' '--- evaluationRun exports and schema symbol ---'
fd -a . web/packages/agenta-entities/src/evaluationRun -t f \
-x sh -c 'printf "\n--- %s ---\n" "$1"; sed -n "1,180p" "$1"' sh {} \
| head -n 900
printf '%s\n' '--- related test import patterns ---'
rg -n 'from ["'\''](`@agenta/entities`|../../src|../../../src)' \
web/packages/agenta-entities/tests/unit web/packages/agenta-entities/src/evaluationRun \
| head -n 200Repository: Agenta-AI/agenta
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- evaluationRun/index.ts ---'
cat -n web/packages/agenta-entities/src/evaluationRun/index.ts
printf '%s\n' '--- evaluationRun/core/index.ts ---'
cat -n web/packages/agenta-entities/src/evaluationRun/core/index.ts
printf '%s\n' '--- schema declarations ---'
rg -n -C3 'evaluationRunsResponseSchema|export .*schema|from ["'\'']\./core|from ["'\'']\./core/schema' \
web/packages/agenta-entities/src/evaluationRunRepository: Agenta-AI/agenta
Length of output: 10156
Import from the public evaluation-run subpath.
Use @agenta/entities/evaluationRun instead of the internal ../../src/evaluationRun/core/schema path. The subpath exports evaluationRunsResponseSchema.
Source: Coding guidelines
Railway Preview Environment
|
Context
After an evaluator was selected for an annotation queue, the annotate view reported that no evaluators were configured. The run API returned the evaluator step correctly, but its source mapping used
kind: "testset". The frontend accepted only an older fixed list of mapping kinds, so Zod rejected the entire run response and the evaluator step disappeared.Changes
Evaluation run mapping kinds now validate as strings, matching the backend contract where this field is free-form. Known mapping kinds remain documented in the TypeScript type for autocomplete. Step types and origins remain strict.
Before, one unfamiliar mapping kind caused the full
/evaluations/runs/queryresponse to fall back to zero runs. After this change, the run remains available and the annotation panel can resolve its evaluator step.A regression test covers a real testcase-backed queue payload with
kind: "testset"and confirms the evaluator reference survives parsing. It also covers future mapping kinds so the same failure mode does not recur.Tests / notes
pnpm lint-fixfromweb/pnpm vitest runinweb/packages/agenta-entities: 93 files, 1,396 tests passedpnpm exec tsc --noEmit -p tsconfig.jsoninweb/packages/agenta-entitiesWhat to QA