Skip to content

chore(evaluator): remove legacy service tree - #231

Merged
SandyChapman merged 1 commit into
remove-evaluator-package-wiring/schapmanfrom
remove-evaluator-service-tree/schapman
Jun 11, 2026
Merged

chore(evaluator): remove legacy service tree#231
SandyChapman merged 1 commit into
remove-evaluator-package-wiring/schapmanfrom
remove-evaluator-service-tree/schapman

Conversation

@SandyChapman

Copy link
Copy Markdown
Contributor

Summary

  • remove the legacy services/evaluator service tree and its pytest/ruff wiring
  • remove legacy evaluator API registration, OpenAPI generation special cases, auth endpoints/scopes, and stale docs/examples
  • refresh platform OpenAPI output and Stainless config so the legacy evaluator API surface is removed from generated specs/config

Validation

  • git diff --check
  • uv run --frozen python services/core/auth/scripts/auth-tools.py check
  • uv run --frozen pytest packages/nmp_platform_runner/tests/test_registry.py services/core/jobs/tests/controllers/test_subprocess_backend.py tools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py services/core/auth/tests/test_embedded_pdp.py -q
  • uv run --frozen ruff check packages/nmp_platform_runner/tests/test_registry.py packages/nmp_platform_runner/src/nmp/platform_runner/registry.py services/core/jobs/tests/controllers/test_subprocess_backend.py script/generate_config_docs.py script/generate_openapi_spec.py script/copyright_fixer.py services/core/auth/scripts/auth-tools.py services/core/auth/tests/test_embedded_pdp.py packages/nmp_testing/src/nmp/testing/client.py plugins/nemo-guardrails/src/nemo_guardrails_plugin/benchmarks/bootstrap.py tools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
  • uv run --frozen ruff format --check packages/nmp_platform_runner/tests/test_registry.py packages/nmp_platform_runner/src/nmp/platform_runner/registry.py services/core/jobs/tests/controllers/test_subprocess_backend.py script/generate_config_docs.py script/generate_openapi_spec.py script/copyright_fixer.py services/core/auth/scripts/auth-tools.py services/core/auth/tests/test_embedded_pdp.py packages/nmp_testing/src/nmp/testing/client.py plugins/nemo-guardrails/src/nemo_guardrails_plugin/benchmarks/bootstrap.py tools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
  • bash tools/lint/lint-openapi.sh
  • make check-policy
  • uv run --frozen pytest plugins/nemo-evaluator/tests -q -k 'not test_example_spec_uses_metric_bundle_shape'

Known Follow-Up / Blocker

  • bash tools/lint/lint-python-sdk.sh still fails because the checked-in generated Python SDK has not been refreshed from Stainless after removing the evaluator OpenAPI schemas/endpoints. The required sdk/stainless.sh sync step pushes the OpenAPI spec/config to Stainless and was blocked by the execution approval guard, so this draft intentionally stops short of regenerated SDK code.
  • bash tools/lint/lint-web-sdk.sh skipped locally because pnpm is not installed.

Stacked on #230.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Documentation preview is ready

Preview: https://nvidia-nemo.github.io/nemo-platform/pr-preview/pr-231/pr-231/

Built from 4d9b888 in workflow run.

This preview is deployed from this PR branch, updates when docs changes are pushed, and will be removed when the PR closes.

@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch 7 times, most recently from 25a3625 to 91518fb Compare June 10, 2026 12:05
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
@SandyChapman
SandyChapman force-pushed the remove-evaluator-service-tree/schapman branch from 4d9b888 to 007e53b Compare June 11, 2026 16:05
@github-actions github-actions Bot added the chore label Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@SandyChapman
SandyChapman marked this pull request as ready for review June 11, 2026 16:10
@SandyChapman
SandyChapman requested review from a team as code owners June 11, 2026 16:10
@SandyChapman
SandyChapman merged commit 3323b9c into remove-evaluator-package-wiring/schapman Jun 11, 2026
6 checks passed
@SandyChapman
SandyChapman deleted the remove-evaluator-service-tree/schapman branch June 11, 2026 17:04
SandyChapman added a commit that referenced this pull request Jun 30, 2026
Re-add queryable persistence of eval-job results (removed with the legacy
service in #231), now on the plugin-job API.

- AgentEvalResultEntity / EvaluateResultEntity store aggregated scores plus
  filterable target/dataset traits; the full bundle stays in the run's fileset
  (bundle_ref). Jobs persist best-effort in run() (a store error never fails
  the eval).
- AgentEvalResult / EvaluateResult API DTOs (mapped from the entities) back the
  read routes, so id/created_at round-trip cleanly on the wire and in the SDK.
- /agent-eval-results and /eval-results list/get/delete routes with trait
  filtering: a DataFilter base translates custom fields to data.* (MetricFilter
  adopts it too, fixing metric_type filtering).
- client.evaluator.{agent_eval_results,eval_results} SDK resources, plus
  metric-type filtering on client.evaluator.metrics.list.
- get_async_task_sdk: async counterpart of get_task_sdk so a sync job run() can
  drive the async entity-store write with the full on-behalf-of identity.

Verified end-to-end on a live platform (agent-eval submit under auth, row-eval
submit, metric-type filtering) plus unit coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman added a commit that referenced this pull request Jun 30, 2026
Re-add queryable persistence of eval-job results (removed with the legacy
service in #231), now on the plugin-job API.

- AgentEvalResultEntity / EvaluateResultEntity store aggregated scores plus
  filterable target/dataset traits; the full bundle stays in the run's fileset
  (bundle_ref). Jobs persist best-effort in run() (a store error never fails
  the eval).
- AgentEvalResult / EvaluateResult API DTOs (mapped from the entities) back the
  read routes, so id/created_at round-trip cleanly on the wire and in the SDK.
- /agent-eval-results and /eval-results list/get/delete routes with trait
  filtering: a DataFilter base translates custom fields to data.* (MetricFilter
  adopts it too, fixing metric_type filtering).
- client.evaluator.{agent_eval_results,eval_results} SDK resources, plus
  metric-type filtering on client.evaluator.metrics.list.
- get_async_task_sdk: async counterpart of get_task_sdk so a sync job run() can
  drive the async entity-store write with the full on-behalf-of identity.

Verified end-to-end on a live platform (agent-eval submit under auth, row-eval
submit, metric-type filtering) plus unit coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman added a commit that referenced this pull request Jun 30, 2026
Re-add queryable persistence of eval-job results (removed with the legacy
service in #231), now on the plugin-job API.

- AgentEvalResultEntity / EvaluateResultEntity store aggregated scores plus
  filterable target/dataset traits; the full bundle stays in the run's fileset
  (bundle_ref). Jobs persist best-effort in run() (a store error never fails
  the eval).
- AgentEvalResult / EvaluateResult API DTOs (mapped from the entities) back the
  read routes, so id/created_at round-trip cleanly on the wire and in the SDK.
- /agent-eval-results and /eval-results list/get/delete routes with trait
  filtering: a DataFilter base translates custom fields to data.* (MetricFilter
  adopts it too, fixing metric_type filtering).
- client.evaluator.{agent_eval_results,eval_results} SDK resources, plus
  metric-type filtering on client.evaluator.metrics.list.
- get_async_task_sdk: async counterpart of get_task_sdk so a sync job run() can
  drive the async entity-store write with the full on-behalf-of identity.

Verified end-to-end on a live platform (agent-eval submit under auth, row-eval
submit, metric-type filtering) plus unit coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman added a commit that referenced this pull request Jul 2, 2026
Re-add queryable persistence of eval-job results (removed with the legacy
service in #231), now on the plugin-job API.

- AgentEvalResultEntity / EvaluateResultEntity store aggregated scores plus
  filterable target/dataset traits; the full bundle stays in the run's fileset
  (bundle_ref). Jobs persist best-effort in run() (a store error never fails
  the eval).
- AgentEvalResult / EvaluateResult API DTOs (mapped from the entities) back the
  read routes, so id/created_at round-trip cleanly on the wire and in the SDK.
- /agent-eval-results and /eval-results list/get/delete routes with trait
  filtering: a DataFilter base translates custom fields to data.* (MetricFilter
  adopts it too, fixing metric_type filtering).
- client.evaluator.{agent_eval_results,eval_results} SDK resources, plus
  metric-type filtering on client.evaluator.metrics.list.
- get_async_task_sdk: async counterpart of get_task_sdk so a sync job run() can
  drive the async entity-store write with the full on-behalf-of identity.

Verified end-to-end on a live platform (agent-eval submit under auth, row-eval
submit, metric-type filtering) plus unit coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
arpitsardhana pushed a commit that referenced this pull request Jul 9, 2026
* feat(evaluator): persist eval results as queryable entities

Re-add queryable persistence of eval-job results (removed with the legacy
service in #231), now on the plugin-job API.

- AgentEvalResultEntity / EvaluateResultEntity store aggregated scores plus
  filterable target/dataset traits; the full bundle stays in the run's fileset
  (bundle_ref). Jobs persist best-effort in run() (a store error never fails
  the eval).
- AgentEvalResult / EvaluateResult API DTOs (mapped from the entities) back the
  read routes, so id/created_at round-trip cleanly on the wire and in the SDK.
- /agent-eval-results and /eval-results list/get/delete routes with trait
  filtering: a DataFilter base translates custom fields to data.* (MetricFilter
  adopts it too, fixing metric_type filtering).
- client.evaluator.{agent_eval_results,eval_results} SDK resources, plus
  metric-type filtering on client.evaluator.metrics.list.
- get_async_task_sdk: async counterpart of get_task_sdk so a sync job run() can
  drive the async entity-store write with the full on-behalf-of identity.

Verified end-to-end on a live platform (agent-eval submit under auth, row-eval
submit, metric-type filtering) plus unit coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator): address review feedback on eval-results persistence

- Best-effort result persistence: wrap persist_agent_eval_result and
  persist_evaluate_result in try/except with a logged warning. The
  authoritative output (bundle/result artifacts) is already saved, so a
  persistence failure no longer fails an otherwise-successful eval job.
  Regression tests cover both jobs.
- Results GET routes now set response_model_exclude_none=True to match
  the list routes' serialization.
- Wrap results GET and DELETE handlers in try/except -> 500 (re-raising
  HTTPException so the 404 is preserved), matching the metrics routes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator): redact credentials from persisted target URLs

target_url is stored on the result entity and returned by the read APIs,
so a target endpoint carrying userinfo or a token query param would leak.
Route both target-field helpers through a new _safe_target_url() that
strips userinfo, redacts sensitive query values, and omits the URL when
it has no host. Tuple shape is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator): use ty rule name in result-entity negative-test suppression

The negative test suppressed with `# type: ignore[call-arg]` (a mypy code
ty doesn't recognize), so ty's `missing-argument` leaked through and failed
the lint-python-types CI check. Use `# ty: ignore[missing-argument]`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

---------

Signed-off-by: Sandy Chapman <schapman@nvidia.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Zenodia pushed a commit to Zenodia/nemo-platform that referenced this pull request Jul 10, 2026
…eMo#515)

* feat(evaluator): persist eval results as queryable entities

Re-add queryable persistence of eval-job results (removed with the legacy
service in NVIDIA-NeMo#231), now on the plugin-job API.

- AgentEvalResultEntity / EvaluateResultEntity store aggregated scores plus
  filterable target/dataset traits; the full bundle stays in the run's fileset
  (bundle_ref). Jobs persist best-effort in run() (a store error never fails
  the eval).
- AgentEvalResult / EvaluateResult API DTOs (mapped from the entities) back the
  read routes, so id/created_at round-trip cleanly on the wire and in the SDK.
- /agent-eval-results and /eval-results list/get/delete routes with trait
  filtering: a DataFilter base translates custom fields to data.* (MetricFilter
  adopts it too, fixing metric_type filtering).
- client.evaluator.{agent_eval_results,eval_results} SDK resources, plus
  metric-type filtering on client.evaluator.metrics.list.
- get_async_task_sdk: async counterpart of get_task_sdk so a sync job run() can
  drive the async entity-store write with the full on-behalf-of identity.

Verified end-to-end on a live platform (agent-eval submit under auth, row-eval
submit, metric-type filtering) plus unit coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator): address review feedback on eval-results persistence

- Best-effort result persistence: wrap persist_agent_eval_result and
  persist_evaluate_result in try/except with a logged warning. The
  authoritative output (bundle/result artifacts) is already saved, so a
  persistence failure no longer fails an otherwise-successful eval job.
  Regression tests cover both jobs.
- Results GET routes now set response_model_exclude_none=True to match
  the list routes' serialization.
- Wrap results GET and DELETE handlers in try/except -> 500 (re-raising
  HTTPException so the 404 is preserved), matching the metrics routes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator): redact credentials from persisted target URLs

target_url is stored on the result entity and returned by the read APIs,
so a target endpoint carrying userinfo or a token query param would leak.
Route both target-field helpers through a new _safe_target_url() that
strips userinfo, redacts sensitive query values, and omits the URL when
it has no host. Tuple shape is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator): use ty rule name in result-entity negative-test suppression

The negative test suppressed with `# type: ignore[call-arg]` (a mypy code
ty doesn't recognize), so ty's `missing-argument` leaked through and failed
the lint-python-types CI check. Use `# ty: ignore[missing-argument]`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>

---------

Signed-off-by: Sandy Chapman <schapman@nvidia.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants