Skip to content

feat(platform): expose state/reason/metadata filters in Run.results()#629

Open
olivermeyer wants to merge 2 commits intomainfrom
feat/filter-run-results
Open

feat(platform): expose state/reason/metadata filters in Run.results()#629
olivermeyer wants to merge 2 commits intomainfrom
feat/filter-run-results

Conversation

@olivermeyer
Copy link
Copy Markdown
Collaborator

@olivermeyer olivermeyer commented May 6, 2026

Why?
Implements PYSDK-128 (originating ticket PAPI-4961). GET /v1/runs/{run_id}/items already accepts state, termination_reason, and custom_metadata (JSONPath) filters, but Run.results() does not surface them — so callers that need filtered results fetch every item and filter client-side, wasting bandwidth and memory on large runs.

How?
Adds three keyword-only parameters (state, termination_reason, custom_metadata) to Run.results() and forwards them to the underlying generated list_run_items_v1_runs_run_id_items_get call only when not None, matching the existing item_ids / external_ids pattern. No codegen regeneration is needed; updates specifications/SPEC_PLATFORM_SERVICE.md and adds five unit tests covering each filter individually, all three combined, and the no-filter omission case.

I'm also removing the --no-build flags from uv sync commands the Dockerfile to avoid failures.

Copilot AI review requested due to automatic review settings May 6, 2026 13:42
@olivermeyer olivermeyer force-pushed the feat/filter-run-results branch from b3040cf to 001d65e Compare May 6, 2026 13:47

This comment was marked as outdated.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines Coverage Δ
src/aignostics/platform/resources/runs.py 85.10% <100.00%> (+0.32%) ⬆️

... and 2 files with indirect coverage changes

@olivermeyer olivermeyer force-pushed the feat/filter-run-results branch from 18c350f to 73a2254 Compare May 6, 2026 14:28
Copy link
Copy Markdown
Collaborator

@akunft akunft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • src/aignostics/platform/CLAUDE.md:812: is missing the new args

I think not testing pagination in the tests for this filter is fine.

Copilot AI review requested due to automatic review settings May 6, 2026 14:49
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread specifications/SPEC_PLATFORM_SERVICE.md Outdated
Comment thread Dockerfile
Comment thread Dockerfile
@olivermeyer olivermeyer force-pushed the feat/filter-run-results branch from dfd959e to d8e84ff Compare May 6, 2026 14:55
@olivermeyer olivermeyer added the claude Trigger Claude Code automation label May 6, 2026
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 6, 2026

@claude
Copy link
Copy Markdown

claude Bot commented May 6, 2026

PR #629 Review Summary

PASS - Ready to merge

All Critical Checks Pass:

  • Test markers: All 5 new tests have @pytest.mark.unit
  • Code quality: make lint passes (ruff, pyright, mypy all green)
  • Conventional commits: Proper format
  • Non-breaking change: Keyword-only parameters with None defaults
  • Architecture compliant: Follows existing patterns

Excellent Implementation:

  • Uses 'is not None' checks for enum filters (prevents subtle bugs)
  • Keyword-only parameters ensure backward compatibility
  • Clean filter building pattern
  • Comprehensive test coverage (5 tests)

Note on Test Timeouts:
Tests timeout due to authentication not being mocked. This is a PRE-EXISTING BUG confirmed on main branch - NOT introduced by this PR. The new tests follow the same pattern as existing tests which also fail. Follow-up issue needed to fix authentication mocking in conftest.py.

See full review details at: /tmp/pr_review.md

Verdict: ✅ claude:review:passed

@claude claude Bot added the claude:review:passed Automated Claude PR review found no blocking issues on the current head commit label May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude:review:passed Automated Claude PR review found no blocking issues on the current head commit claude Trigger Claude Code automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants