fix(eval): update openapi spec - #1176
Conversation
81240d9 to
fcc7045
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesAggregate score support
OpenAPI schema generation
Sequence Diagram(s)sequenceDiagram
participant Evaluator API
participant AgentEvalAggregateScore
participant comparisonScoresForAgentEval
participant EvalAggregateScoresTable
Evaluator API->>AgentEvalAggregateScore: returns scalar aggregate score
AgentEvalAggregateScore->>comparisonScoresForAgentEval: provides value
comparisonScoresForAgentEval->>EvalAggregateScoresTable: normalizes score
EvalAggregateScoresTable->>EvalAggregateScoresTable: sorts and renders value
sequenceDiagram
participant Platform FastAPI app
participant OpenAPI hook
participant Query schema registry
participant OpenAPI document
Platform FastAPI app->>OpenAPI hook: generates specification
OpenAPI hook->>Query schema registry: registers query schemas
Query schema registry->>OpenAPI document: promotes definitions and rewrites references
OpenAPI hook->>Platform FastAPI app: returns cached specification
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-evaluator/openapi/openapi.yaml (1)
2738-2740: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftVersion or migrate the optional
countresponse change.The v2 contract previously required
count, but these responses can now omit it when the count is unknown. Existing generated clients can reject such responses. Preserve compatibility or provide a coordinated client migration for bothAggregateRangeScoreandAggregateRubricScore.🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml` around lines 2738 - 2740, Update both AggregateRangeScore at plugins/nemo-evaluator/openapi/openapi.yaml:2738-2740 and AggregateRubricScore at plugins/nemo-evaluator/openapi/openapi.yaml:2823-2826 to version the optional count response change or coordinate migration of generated clients. Ensure the v2 contract remains compatible with responses that omit count when unknown, while preserving required name and nan_count fields.Source: Linters/SAST tools
🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 2668-2673: Update the count descriptions in all three schemas:
plugins/nemo-evaluator/openapi/openapi.yaml lines 2668-2673, 2751-2756, and
2835-2841. Replace the wording that says count is “None when the sample size is
unknown” with wording that says it is “omitted when the sample size is unknown,”
while keeping the integer type and remaining description unchanged.
---
Outside diff comments:
In `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 2738-2740: Update both AggregateRangeScore at
plugins/nemo-evaluator/openapi/openapi.yaml:2738-2740 and AggregateRubricScore
at plugins/nemo-evaluator/openapi/openapi.yaml:2823-2826 to version the optional
count response change or coordinate migration of generated clients. Ensure the
v2 contract remains compatible with responses that omit count when unknown,
while preserving required name and nan_count fields.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e5dc2ab5-a7ee-4cab-9ba9-9115b935cd03
📒 Files selected for processing (1)
plugins/nemo-evaluator/openapi/openapi.yaml
|
fcc7045 to
4bc08dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.py`:
- Around line 298-301: Align the count field’s documentation and serialization
behavior in the results model: since SDK JSON paths use model_dump(mode="json")
without exclude_none=True, ensure unknown counts are documented as serialized as
null, or update all relevant serialization paths to consistently omit None
values. Add or update tests to verify the resulting JSON payload shape.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 69bed69c-6f2d-4585-b633-4f6e43a73832
📒 Files selected for processing (5)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.pyweb/packages/studio/src/api/evaluation/agent-evaluations.tsweb/packages/studio/src/components/dataViews/EvalComparisonTable/utils.test.tsweb/packages/studio/src/components/dataViews/EvalComparisonTable/utils.tsweb/packages/studio/src/components/evaluation/EvalAggregateScoresTable.tsx
4bc08dd to
d96fef3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 2710-2720: Align nullability and descriptions for sample_std_dev
and sample_variance in plugins/nemo-evaluator/openapi/openapi.yaml at lines
2710-2720, 2793-2803, and 2878-2888. Check the corresponding serialization
behavior, then either change each description’s “None” wording to “Omitted” when
unavailable values are omitted, or allow JSON null consistently in all six
schemas when nulls are serialized.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6bc36405-0aa7-4c98-a093-8159731f3ebc
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/results.pyis excluded by!sdk/**
📒 Files selected for processing (1)
plugins/nemo-evaluator/openapi/openapi.yaml
d96fef3 to
277a69c
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
tools/nemo-platform-stress/pyproject.toml (1)
14-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo
[tool.uv.sources]entries are unused.
nemo-platform-pluginandnmp-build-toolsare not independenciesor[dependency-groups]. Remove them or add the matching dependencies.🤖 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 `@tools/nemo-platform-stress/pyproject.toml` around lines 14 - 17, Remove the unused nemo-platform-plugin and nmp-build-tools entries from [tool.uv.sources], unless corresponding entries are added to dependencies or [dependency-groups]. Keep the nemo-platform-sdk source unchanged.tools/nemo-platform-stress/tests/test_runner.py (1)
26-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare
bodieson the server class.
server.bodiesis set dynamically at Line 172 and read at Line 53. Type checking withtycan report an unknown attribute onsocketserver.ThreadingTCPServer. Declare it on_ThreadedTCPServer.♻️ Proposed fix
class _ThreadedTCPServer(socketserver.ThreadingTCPServer): allow_reuse_address = True daemon_threads = True + bodies: list[bytes] = [] + + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.bodies = []Also applies to: 53-53, 172-172
🤖 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 `@tools/nemo-platform-stress/tests/test_runner.py` around lines 26 - 29, Declare the bodies attribute on _ThreadedTCPServer with an appropriate type annotation and initialize it as needed, so accesses at the server request-handling code and its assignment later are recognized by ty.Source: Coding guidelines
tools/nemo-platform-stress/src/nemo_platform_stress/builtin_tests.py (1)
240-241: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueProvider validation happens per iteration.
An invalid
--job-providerfails every iteration with the sameValueErrorinstead of exiting early. Validate the value incli.pybefore the run starts.🤖 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 `@tools/nemo-platform-stress/src/nemo_platform_stress/builtin_tests.py` around lines 240 - 241, Move --job-provider validation out of the per-iteration logic in builtin_tests.py and perform it once in cli.py before starting the run. Preserve the allowed values subprocess, cpu, and gpu, and retain the existing ValueError message or equivalent validation behavior for invalid input.tools/jobs_stress.py (1)
888-896: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winA new
NeMoPlatformclient is constructed per result in the thread pool.
_fetch_result_logsand_fetch_result_task_status_detailscallmake_client(config)for every job. Each client opens its own connection pool and is never closed. For large runs this wastes sockets. Create one client per worker or one shared client and pass it in.Also applies to: 947-959
🤖 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 `@tools/jobs_stress.py` around lines 888 - 896, The result-fetching workflow currently creates an unclosed NeMoPlatform client for each job. Update _fetch_result_logs and _fetch_result_task_status_details to receive and reuse a single client created per worker or shared across the thread pool, and ensure that client is closed when the workflow completes.tools/nemo-platform-stress/src/nemo_platform_stress/runner.py (1)
650-655: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueError message names only one caller.
_run_asyncalso backsrun_suite. Make the message generic so arun_suitefailure is not misattributed.♻️ Proposed fix
- raise RuntimeError("run_staged_connections cannot be called from an active asyncio event loop") + raise RuntimeError("this entry point cannot be called from an active asyncio event loop; use the async variant")🤖 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 `@tools/nemo-platform-stress/src/nemo_platform_stress/runner.py` around lines 650 - 655, Update the RuntimeError message raised by _run_async to describe that the helper cannot be called from an active asyncio event loop, without naming only run_staged_connections; keep the existing loop detection and asyncio.run behavior unchanged so both run_staged_connections and run_suite failures are accurately represented.tools/nemo-platform-stress/README.md (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd prerequisites at the top and a "Next Steps" section at the end.
The coding guidelines require both for documentation pages. State the required
uvversion, the running platform deployment, and the credentials before the first command. Add cross-links to the SDK and jobs documentation at the end.Based on learnings from the coding guidelines: "Always list prerequisites at the top of documentation pages before other content" and "Include 'Next Steps' section at the end with cross-links to related documentation content".
🤖 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 `@tools/nemo-platform-stress/README.md` around lines 1 - 8, Add a prerequisites section at the beginning of the README before the existing description, specifying the required uv version, a running NeMo Platform deployment, and valid credentials before any commands. Append a “Next Steps” section at the end with links to the SDK and jobs documentation.Source: Coding guidelines
tools/lint/lint-openapi.sh (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport every stale spec, not just the first.
set -eaborts the loop on the first non-zerodiff. With many specs now discovered dynamically, a contributor sees one failure, fixes it, re-runs, and finds the next. Accumulate failures and report them together.Proposed change
+stale=0 for spec_file in "${spec_files[@]}"; do - diff "${check_dir}/${spec_file}" "${spec_file}" + if ! diff -u "${check_dir}/${spec_file}" "${spec_file}"; then + stale=1 + fi done +if [[ "${stale}" -ne 0 ]]; then + echo "OpenAPI specs are out of date. Run script/generate-openapi-spec.sh and commit the result." >&2 + exit 1 +fi🤖 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 `@tools/lint/lint-openapi.sh` around lines 20 - 22, Update the spec_file validation loop so a non-zero diff does not terminate iteration; accumulate whether any comparisons fail while continuing to check every discovered spec, then return a non-zero status after the loop if failures occurred..github/actions/changes/action.yaml (1)
76-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the OpenAPI lint script to the filter.
tools/lint/lint-openapi.shchanges in this PR but is not in theopenapifilter. A change to that script will not trigger the OpenAPI check. Thehelmfilter already liststools/lint/lint-helm.shby the same reasoning.The output description on line 8 is now stale. It still says "any file under openapi/".
Proposed change
- 'script/generate_openapi_spec.py' - 'script/generate-openapi-spec.sh' - 'script/openapi_helper/**' + - 'tools/lint/lint-openapi.sh' + - '.github/actions/changes/action.yaml'Also update line 8:
- description: "'true' if any file under openapi/ changed" + description: "'true' if any OpenAPI spec, generator, or spec source input changed"🤖 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 @.github/actions/changes/action.yaml around lines 76 - 84, Update the openapi filter in the changes action configuration to include tools/lint/lint-openapi.sh, ensuring edits to the lint script trigger the OpenAPI check. Also revise the filter’s output description to describe the current OpenAPI-related paths instead of claiming it covers every file under openapi/.
🤖 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 `@plugins/nemo-auditor/openapi/openapi.yaml`:
- Around line 1880-1954: Fix register_query_param_schemas in
script/openapi_helper so generated filter properties reference their local $defs
(or consistently promote DatetimeFilter and StringFilter into components.schemas
while removing local definitions), eliminating dangling references. Regenerate
all affected filters in plugins/nemo-auditor/openapi/openapi.yaml lines
1880-1954 and plugins/nemo-anonymizer/openapi/openapi.yaml lines 1726-1800,
including every filter schema listed in the review and the live job filter query
parameters.
In `@tools/jobs_stress.py`:
- Around line 2383-2390: The Docker event-capture condition around
start_docker_events_capture must also activate for runs targeting a Docker
provider profile, even when backends is empty because --backend was omitted.
Update the condition to recognize the relevant docker-provider-profile
configuration while preserving the existing backend-based behavior.
- Around line 828-841: Update the polling logic around the pending-status branch
so both the global timeout from submitted_at_monotonic/config.timeout_seconds
and the pending image-pull deadline are evaluated on every poll. Preserve the
existing JobTimeoutError messages and ensure the earliest exceeded deadline
raises without letting pending status bypass --timeout.
In `@tools/nemo-platform-stress/.gitignore`:
- Line 5: Remove uv.lock from the ignore entries in the project’s .gitignore so
the lockfile is tracked and committed. Keep it synchronized with pyproject.toml
using uv lock operations.
In `@tools/nemo-platform-stress/src/nemo_platform_stress/runner.py`:
- Around line 679-687: Update serialize_settings to sanitize
payload["client"]["headers"] before returning the report, redacting sensitive
credential values such as Authorization while preserving non-sensitive headers
and the existing access_token/config_path handling.
---
Nitpick comments:
In @.github/actions/changes/action.yaml:
- Around line 76-84: Update the openapi filter in the changes action
configuration to include tools/lint/lint-openapi.sh, ensuring edits to the lint
script trigger the OpenAPI check. Also revise the filter’s output description to
describe the current OpenAPI-related paths instead of claiming it covers every
file under openapi/.
In `@tools/jobs_stress.py`:
- Around line 888-896: The result-fetching workflow currently creates an
unclosed NeMoPlatform client for each job. Update _fetch_result_logs and
_fetch_result_task_status_details to receive and reuse a single client created
per worker or shared across the thread pool, and ensure that client is closed
when the workflow completes.
In `@tools/lint/lint-openapi.sh`:
- Around line 20-22: Update the spec_file validation loop so a non-zero diff
does not terminate iteration; accumulate whether any comparisons fail while
continuing to check every discovered spec, then return a non-zero status after
the loop if failures occurred.
In `@tools/nemo-platform-stress/pyproject.toml`:
- Around line 14-17: Remove the unused nemo-platform-plugin and nmp-build-tools
entries from [tool.uv.sources], unless corresponding entries are added to
dependencies or [dependency-groups]. Keep the nemo-platform-sdk source
unchanged.
In `@tools/nemo-platform-stress/README.md`:
- Around line 1-8: Add a prerequisites section at the beginning of the README
before the existing description, specifying the required uv version, a running
NeMo Platform deployment, and valid credentials before any commands. Append a
“Next Steps” section at the end with links to the SDK and jobs documentation.
In `@tools/nemo-platform-stress/src/nemo_platform_stress/builtin_tests.py`:
- Around line 240-241: Move --job-provider validation out of the per-iteration
logic in builtin_tests.py and perform it once in cli.py before starting the run.
Preserve the allowed values subprocess, cpu, and gpu, and retain the existing
ValueError message or equivalent validation behavior for invalid input.
In `@tools/nemo-platform-stress/src/nemo_platform_stress/runner.py`:
- Around line 650-655: Update the RuntimeError message raised by _run_async to
describe that the helper cannot be called from an active asyncio event loop,
without naming only run_staged_connections; keep the existing loop detection and
asyncio.run behavior unchanged so both run_staged_connections and run_suite
failures are accurately represented.
In `@tools/nemo-platform-stress/tests/test_runner.py`:
- Around line 26-29: Declare the bodies attribute on _ThreadedTCPServer with an
appropriate type annotation and initialize it as needed, so accesses at the
server request-handling code and its assignment later are recognized by ty.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 993eab00-21d9-417b-9811-f7c465348c48
📒 Files selected for processing (19)
.github/actions/changes/action.yaml.github/workflows/ci.yamlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.pypackages/nemo_evaluator_sdk/tests/values/test_results.pyplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-anonymizer/openapi/openapi.yamlplugins/nemo-auditor/openapi/openapi.yamltools/jobs_stress.pytools/lint/lint-openapi.shtools/nemo-platform-stress/.gitignoretools/nemo-platform-stress/README.mdtools/nemo-platform-stress/pyproject.tomltools/nemo-platform-stress/src/nemo_platform_stress/__main__.pytools/nemo-platform-stress/src/nemo_platform_stress/builtin_tests.pytools/nemo-platform-stress/src/nemo_platform_stress/cli.pytools/nemo-platform-stress/src/nemo_platform_stress/runner.pytools/nemo-platform-stress/tests/test_builtin_tests.pytools/nemo-platform-stress/tests/test_runner.pytools/tests/test_jobs_stress.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.py
96095d8 to
a7cce2c
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-evaluator/openapi/openapi.yaml (1)
2737-2740: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAllow
nullforcount.
countis optional and serialization emitscount: nullwhen unknown. UpdateAggregateRangeScore.countandAggregateRubricScore.countto allownull; do not makecountrequired again. Apply the same fix toAggregateScalarScore.count.🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml` around lines 2737 - 2740, Update AggregateRangeScore.count, AggregateRubricScore.count, and AggregateScalarScore.count in plugins/nemo-evaluator/openapi/openapi.yaml at lines 2737-2740 and 2821-2826 to allow null while keeping count optional and not adding it to required. Ensure each schema reflects serialized count: null values.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 2737-2740: Update AggregateRangeScore.count,
AggregateRubricScore.count, and AggregateScalarScore.count in
plugins/nemo-evaluator/openapi/openapi.yaml at lines 2737-2740 and 2821-2826 to
allow null while keeping count optional and not adding it to required. Ensure
each schema reflects serialized count: null values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a53320d7-eac3-4a16-9d23-9ad6db406597
📒 Files selected for processing (2)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.pyplugins/nemo-evaluator/openapi/openapi.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.py
47caef9 to
6fa750b
Compare
6fa750b to
419276b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugins/nemo-evaluator/openapi/openapi.yaml (1)
2668-2673: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAllow documented
nullvalues forcount.Each description says
countserializes asnull, buttype: integerrejectsnull. Update the source schema to accept integer or null, then regenerate the specification. Otherwise, these documented responses violate the API contract.
plugins/nemo-evaluator/openapi/openapi.yaml#L2668-L2673: AllownullforAggregateRangeScore.count.plugins/nemo-evaluator/openapi/openapi.yaml#L2751-L2756: AllownullforAggregateRubricScore.count.plugins/nemo-evaluator/openapi/openapi.yaml#L2835-L2841: AllownullforAggregateScalarScore.count.As per coding guidelines, “Validate and regenerate the OpenAPI specification using the repository’s OpenAPI generation commands when API definitions change.”
🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml` around lines 2668 - 2673, Update count in AggregateRangeScore (plugins/nemo-evaluator/openapi/openapi.yaml:2668-2673), AggregateRubricScore (plugins/nemo-evaluator/openapi/openapi.yaml:2751-2756), and AggregateScalarScore (plugins/nemo-evaluator/openapi/openapi.yaml:2835-2841) to accept integer or null, matching their documented serialization behavior. Validate the source schema and regenerate the OpenAPI specification using the repository’s standard generation commands.Source: Coding guidelines
🤖 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/nemo_platform_plugin/src/nemo_platform_plugin/jobs/openapi_utils.py`:
- Around line 163-168: Update custom_openapi() so registration failures from
register_query_param_schemas(default_openapi()) clear app.openapi_schema before
re-raising the original exception, allowing subsequent calls to retry
registration. Add a test covering two app.openapi() calls after a component
conflict and verify registration is attempted again.
---
Duplicate comments:
In `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 2668-2673: Update count in AggregateRangeScore
(plugins/nemo-evaluator/openapi/openapi.yaml:2668-2673), AggregateRubricScore
(plugins/nemo-evaluator/openapi/openapi.yaml:2751-2756), and
AggregateScalarScore (plugins/nemo-evaluator/openapi/openapi.yaml:2835-2841) to
accept integer or null, matching their documented serialization behavior.
Validate the source schema and regenerate the OpenAPI specification using the
repository’s standard generation commands.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0ff5aa8a-76c4-4b7d-bc90-7857cbe57189
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/results.pyis excluded by!sdk/**
📒 Files selected for processing (10)
.github/workflows/ci.yamlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/openapi_utils.pypackages/nmp_common/src/nmp/common/api/utils.pypackages/nmp_common/tests/api/test_query_param_schemas.pypackages/nmp_platform_runner/src/nmp/platform_runner/server.pypackages/nmp_platform_runner/tests/test_server.pyplugins/nemo-evaluator/openapi/openapi.yamlscript/openapi_helper/plugin_loader.pytests/unit/test_plugin_openapi_loader.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yaml
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/results.py
419276b to
d5d63de
Compare
d5d63de to
68b294f
Compare
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
68b294f to
7c01f7f
Compare
Summary
Related Issue
Changes
Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
Summary by CodeRabbit
New Features
Bug Fixes
Tests