Repository navigation
feat(results): share execution scope between value and resource filters - #977
Conversation
Allows resource summaries and measurements to count only each group's latest execution. This answers what one run of a diagnostic costs, while the default still totals the compute spent.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Change: Feature Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 5 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarises the main change: sharing execution-scope handling between value and resource filters. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description is detailed and explains the scope, behaviour changes, defaults, exceptions, and compatibility impact. It does not use the required ## Description and ## Checklist heading… Full details: Description check
-
Explanation
The pull request description is detailed and explains the scope, behaviour changes, defaults, exceptions, and compatibility impact. It does not use the required ## Description and ## Checklist headings, and it does not explicitly confirm each checklist item. The changed files show that tests and a changelog item were added.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 5 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ilters Adds ExecutionScope, holding include_retracted and latest_only, and apply_execution_scope. MetricValueFilter, ResourceFilter and with_values now apply retraction and latest the same way. ResourceFilter gains include_retracted, so summaries and measurements skip retracted executions by default. profiles keeps every sample, retracted ones included.
lewisjared
changed the title
Retracted executions are now excluded everywhere unless a caller opts in. ResourcesReader.measurement still finds one by id, and profiles accepts a ResourceFilter. Also tidies the filter docstrings, treats an empty slug as a constraint, and tests stats and promoted scoping.
1 participant
Adds
latest_onlyandinclude_retractedtoResourceFilter, through a sharedExecutionScopethatMetricValueFilternow uses too. One helper,apply_execution_scope, applies both to any select joined toExecution, so values, resources andDiagnosticsReader.with_valuestreat retraction and "latest" the same way.The defaults answer how much compute was spent, retries and failures included.
latest_onlyanswers what one run of a diagnostic costs, because old retries no longer inflate the total and quick failures no longer drag down the mean. ref-app currently mixes the two: the diagnostic page totals every execution, while the catalog counts only the promoted version.Retracted executions are excluded by default everywhere, unless a caller passes
include_retracted=True. The one exception isResourcesReader.measurement(execution_id), because a lookup by id names the execution.Behaviour changes:
ResourcesReader.summary,summaries,measurementsandprofilesnow skip retracted executions by default.ResourcesReader.profilesaccepts aResourceFilter, so it takeslatest_only,include_retractedand the exact slugs. The existing keyword arguments still work, but cannot be combined with a filter.MetricValueFilter(diagnostic_slug="")now matches nothing, likeResourceFilter, instead of ignoring the slug.MetricValueFilter(latest_only=True, include_retracted=True)now joinsExecution. The rows returned are the same.latest_onlypicks the latest execution before retracted ones are dropped. So if a group's newest execution is retracted, that group gives nothing unlessinclude_retracted=True. It never falls back to an older execution, because the newer one superseded it.DatasetFilterdrops retracted rows first instead, because an older dataset version is still valid.Execution statistics and
ExecutionsReader.groupsstill ignore retraction, because they report run status rather than results.ResourceFiltercounts every diagnostic version. Nothing setsExecution.retractedyet, so none of this changes current output.