You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Require every configured validation and test metric before accepting a fitted result or reference. Compare each prediction with the sampled input population, record requested/scored/skipped counts measured from each output, and reject unexpected skips. A population error names the failing prediction. A reference without the configured metrics fails before ingest.
Resolve forecast coordinates from the scoring snapshot metadata. The requested anchors and the output coordinates use one rule. The requested-to-output comparison uses timestamp values, so nanosecond input and microsecond output storage match.
The four-file patch has 163 additions and 20 deletions, including an 87-line regression test file.
34 repository CPU checks passed on pandas 3.1.1 and pandas 2.3.3: python -m pytest -q -c /dev/null --confcutdir=test/scripts -p no:cacheprovider test/scripts/test_engine_e2e_case_results.py test/scripts/test_engine_e2e_case_hardware.py.
The OSS Readiness test list passed 189 tests with pandas 3.1.1 and no PyArrow. test_history_scan_explicitly_selects_all_commits failed because the local checkout is shallow.
CPU runs of the runner on tiny binary, multiclass, temporal_link and static_link graphs passed. Each run recorded 20/20/0 counts for the first, repeat and reload predictions. A binary run with a failed reload manifest write also passed. A binary run with a stale reference failed before ingest with Missing or non-finite reference metrics, and a run with a matching reference passed.
The retained 117-case suite from the larger earlier patch passed 102 cases after one call-site update and one message update. The other 15 cases expect behavior that this patch removes on purpose. Twelve cases read copied manifest counts. Three cases accept different timestamp units between runs on pandas 2.3.3.
Flake8, isort and git diff --check passed.
GPU training and the full scale campaign were not rerun.
No actionable comments were generated in the recent review.
Walkthrough
The engine case helpers now validate configured validation and test metrics, prediction counts, and task-specific prediction coordinates against the requested population. The changes add end-to-end tests for metric and population checks, and add the test module to the OSS readiness workflow.
Sequence Diagram(s)
sequenceDiagram
participant run
participant prediction_table
participant prediction
participant check_predictions
run->>prediction_table: derive requested rows and task-specific coordinates
prediction_table-->>run: return requested rows and coordinates
run->>prediction: generate predictions and record population counts
prediction-->>run: return prediction results
run->>check_predictions: check repeated and reloaded predictions against requested rows and coordinates
Loading
Merge Risk: Minimal · up to 6c78b
The change tightens result checks; no merge-blocking risk was established.
Security architecture risk: Low · up to df833
The changes strengthen test-result validation without adding a network entrypoint, privileges, or a new data-access path. No introduced security concern was substantiated. Assurance remains bounded because full training runs were not repeated and security coverage is incomplete.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The changed behavior is reachable through existing case scripts and local regression tests. Comparing base and head shows validation changes around existing snapshot, local input-file, model-prediction, and output paths, rather than an expansion of caller authority or a new sensitive sink.
Trust Boundaries and Controls
observed — The added controls bind prediction acceptance to held-out input identities, reject skipped input populations, and require configured finite validation/test metrics before acceptance. Forecast coordinate selection uses the scoring snapshot metadata rather than inferring the time field solely from returned columns.
Resilience and Maintainability Implications
observed — Failure remains the default result state. The exception/finally path records failure, restores signal handling, stops the observer, writes the result, and attempts temporary-directory cleanup. Direct result writing is non-atomic in both base and head; this is a pre-existing runner limitation, not a regression introduced by the validation changes.
Pre-merge checks: 2 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Change Map Design Review
The marked PR conversation comment and linked problem are unavailable. The PR description links to the change map but does not include its contents. The diff contains 163 additions and 20 deletions, a…
Provide the conversation comment starting with <!-- kumo-pr-change-map --> and the linked problem or release requirement. Review them against the diff and existing code.
Passed checks: 2 passed
Check name
Status
Explanation
Pr Size
The pull request changes 4 files. The limit is 40 files.
Public Resource Access
The changed lines add no external resource references. The added file paths refer to files in this repository.
Full details: Change Map Design Review
Explanation
The marked PR conversation comment and linked problem are unavailable. The PR description links to the change map but does not include its contents. The diff contains 163 additions and 20 deletions, and the repository template requires the marked comment. These sources do not establish the comment's design rationale or stopping condition.
Resolution
Provide the conversation comment starting with <!-- kumo-pr-change-map --> and the linked problem or release requirement. Review them against the diff and existing code.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between df833ef and 6c78b57.
Files selected for processing: 3
packages/kumo-engine/test/cases/README.md
packages/kumo-engine/test/cases/common.py
test/scripts/test_engine_e2e_case_results.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
Before this patch, the runner did not reject a missing test metric. It also did not reject three identical outputs that each omitted the same requested input. Its fixed forecast keys also omitted the entity's configured time column.
File
Function / scope
What changed
Why
packages/kumo-engine/test/cases/common.py
prediction_table, check_metrics, check_population, prediction_coordinates, check_predictions, run
Retain sampled input keys from the coordinate rule. Require configured validation/test metrics, and check references before ingest. Count each output's population and check its coordinates. Resolve the forecast clock from metadata.
Reject incomplete results before pass status or reference publication. Compare requested and output timestamp values across storage units.
test/scripts/test_engine_e2e_case_results.py
Metric, population and coordinate regressions
Add five compact CPU cases, including missing rows, foreign rows, measured counts, task coordinate rules, reordered forecasts and timestamp precision.
Reproduce the three gaps without GPU or cloud dependencies.
.github/workflows/oss-readiness.yml
Repository CPU checks
Include the new regression file.
Run these guards in existing PR CI.
packages/kumo-engine/test/cases/README.md
Result acceptance contract
Document required metrics, complete sampled populations, measured counts, zero skips and forecast coordinates.
Make the stricter reference and candidate checks clear.
Verification: 34 repository CPU checks passed on pandas 3.1.1 and 2.3.3. CPU runner runs passed for binary, multiclass, temporal_link and static_link. A stale reference failed before ingest. Flake8, isort and whitespace checks passed. The description records the retained-suite results and one existing shallow-history failure.
Scope ends at these shared runner guards and focused regressions. Training budgets, learning thresholds and prediction tolerances stay unchanged.
The reason will be displayed to describe this comment to others. Learn more.
Nitpick comments: 2
packages/kumo-engine/test/cases/common.py: 2
340-342: Trivial · Performance & Scalability · Low value
Timestamp comparison does not match the comment.
The comment says the code compares timestamp values across ns/us storage units. The code compares Index.tolist() values. Pandas Timestamp objects compare equal across units, so this likely works. Mixed-unit MultiIndex levels may behave differently when sort_index() runs. A us frame and an ns frame could sort differently if a level contains mixed types.
The test at lines 68-69 exercises this path with a MultiIndex. Confirm that it passes on the pinned pandas 3.x. If it passes, no change is needed.
Also, tolist() on a large MultiIndex builds a Python tuple list. This can use large amounts of memory for large prediction populations. Consider a vectorized comparison.
275-282: Trivial · Functional Correctness · Low value
check_metrics has a wrong error message and an unreachable guard.
The error lists only invalid names. Missing and non-finite names are mixed. This is acceptable. But not required or not metrics makes an empty configured_metrics list raise with an empty list as detail. That message gives no cause.
Also, math.isfinite(value) raises TypeError for non-numeric values. Reference JSON could contain null. That raises TypeError, not ValueError.
Reviewing files that changed from the base of the PR and between 8b4942f and f90e9f7.
Files selected for processing: 4
.github/workflows/oss-readiness.yml
packages/kumo-engine/test/cases/README.md
packages/kumo-engine/test/cases/common.py
test/scripts/test_engine_e2e_case_results.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
Count requested, scored and skipped anchors from each prediction frame.
Engine manifests copy the shared input table counts, and the engine
writes them on a best-effort basis. A population error now names the
failing prediction and gives its counts. DataFrame inputs work again.
Derive the requested anchors and the extra output fields from
prediction_coordinates. One rule now controls the static_link anchor.
CPU tests cover the binary, link and static_link coordinate rules.
Restore the index comparison between runs. All three runs read the same
microsecond output files. The requested-to-output check still compares
nanosecond and microsecond timestamps by value.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
The unit test does not cover mixed ns and us units across outputs.
All three frames use microseconds here, and the requested table uses nanoseconds. The test shows that population matching is value-based. It does not show that two outputs in different units pass the index.equals check in check_predictions. Add one case with one output in ns and one in us.
Reviewing files that changed from the base of the PR and between f90e9f7 and df833ef.
Files selected for processing: 3
packages/kumo-engine/test/cases/README.md
packages/kumo-engine/test/cases/common.py
test/scripts/test_engine_e2e_case_results.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
Every case passes its metrics through overrides, so the runner can check
the reference right after it loads it. A stale reference now fails in
seconds, before ingest and training-table generation. The error names
the reference instead of training. A case without metric overrides
still fails at the end-of-run reference comparison.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Supersedes #302.
Require every configured validation and test metric before accepting a fitted result or reference. Compare each prediction with the sampled input population, record requested/scored/skipped counts measured from each output, and reject unexpected skips. A population error names the failing prediction. A reference without the configured metrics fails before ingest.
Resolve forecast coordinates from the scoring snapshot metadata. The requested anchors and the output coordinates use one rule. The requested-to-output comparison uses timestamp values, so nanosecond input and microsecond output storage match.
The four-file patch has 163 additions and 20 deletions, including an 87-line regression test file.
Change map
File-by-file change map.
Testing Done
python -m pytest -q -c /dev/null --confcutdir=test/scripts -p no:cacheprovider test/scripts/test_engine_e2e_case_results.py test/scripts/test_engine_e2e_case_hardware.py.test_history_scan_explicitly_selects_all_commitsfailed because the local checkout is shallow.Missing or non-finite reference metrics, and a run with a matching reference passed.git diff --checkpassed.GPU training and the full scale campaign were not rerun.