fix: clamp _epoch_to_iso to representable range; repair regen tests

[yilu331]

Summary

Found by /check-and-test (nightly CI). Two related fixes surfaced by the new evaluation-overview query added in #168.

Application fix — clamp _epoch_to_iso to a representable range:

• _epoch_to_iso raised ValueError when callers passed sentinel "open" window bounds (e.g. to_ts=10**12, used by the evaluation-overview API as a far-future upper bound) because datetime.fromtimestamp cannot represent year > 9999.
• The new get_agent_success_evaluation_results_in_window query (fix: optimize evaluation overview reads #168) hit this and 500'd the evaluation overview endpoint on a fresh app.
• Clamp the input to [0, 253402300799] centrally in _epoch_to_iso so all current and future callers are protected. _shadow_verdicts.py had already worked around this at two call sites with a local _MAX_SAFE_EPOCH_TS clamp; that constant now lives in _base.py and the redundant per-call clamps are removed.

Test fix — stale drift from #168:

• The force_regenerate runner now captures prior result ids via the targeted get_agent_success_evaluation_result_ids lookup instead of reading full result rows. test_group_evaluation_runner_regen.py still mocked the old method, so the by-id delete asserted the wrong argument (MagicMock tests) or bound a MagicMock into SQL (real-sqlite tests). Seed the new lookup and patch get_extractor_name so the capture query's evaluation_name matches the seeded rows.

Changes

• reflexio/services/storage/sqlite_storage/_base.py — central clamp in _epoch_to_iso; host _MAX_SAFE_EPOCH_TS.
• reflexio/services/storage/sqlite_storage/_shadow_verdicts.py — drop the now-redundant local clamps.
• tests/.../test_group_evaluation_runner_regen.py — align mocks with the by-id lookup path.

Test Plan

• test_group_evaluation_runner_regen.py passes under both MagicMock and real-sqlite paths.
• Evaluation-overview endpoint no longer 500s with an open far-future to_ts on a fresh app.

Provenance

Generated by the nightly AI-driven CI run on 2026-06-18. The enterprise-side submodule bump + companion test changes are in ReflexioAI/reflexio-enterprise#263 — that PR's open_source/reflexio pointer should be repointed to this PR's merged main commit once it lands.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Fixed query failures when requesting results with extreme or boundary timestamp values by safely handling out-of-range epochs in SQLite-backed storage.

• Tests

  • Improved coverage for agent success evaluation “force regenerate” flows, including correct identity matching and deletion/preservation behavior across both mocked and real SQLite scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f3bbc3f0-a4b1-447d-bceb-486d6d0ca873

📥 Commits

Reviewing files that changed from the base of the PR and between c077b71 and d757e0a.

📒 Files selected for processing (1)

• tests/server/services/agent_success_evaluation/test_group_evaluation_runner_regen.py

---

📝 Walkthrough

Walkthrough

_epoch_to_iso in _base.py now clamps Unix timestamps to a safe representable range via two new module-level constants, removing the need for identical inline clamping in _shadow_verdicts.py. Tests for the force_regenerate runner are updated to mock get_agent_success_evaluation_result_ids with prior-result IDs and patch get_extractor_name to "overall_success" across unit and end-to-end SQLite tests.

Changes

Centralize epoch clamping in _epoch_to_iso

Layer / File(s)	Summary
Add safe-range constants and clamp in _epoch_to_iso reflexio/server/services/storage/sqlite_storage/_base.py	Introduces _MIN_SAFE_EPOCH_TS = 0 and _MAX_SAFE_EPOCH_TS = 253_402_300_799, then clamps the incoming ts argument to that range inside _epoch_to_iso before calling datetime.fromtimestamp.
Remove redundant clamping from shadow verdict queries reflexio/server/services/storage/sqlite_storage/_shadow_verdicts.py	Deletes the local _MAX_SAFE_EPOCH_TS constant and rewrites from_iso/to_iso in both get_shadow_comparison_verdicts and get_recent_shadow_comparison_verdicts to call _epoch_to_iso directly without inline bounds expressions.

Fix regen runner test wiring

Layer / File(s)	Summary
Mock ID-capture path in _make_storage tests/server/services/agent_success_evaluation/test_group_evaluation_runner_regen.py	_make_storage now seeds storage.get_agent_success_evaluation_result_ids with a side_effect function that filters prior_results by (session_id, evaluation_name, agent_version) to extract matching result_id values.
Update force_regenerate and SQLite tests with extractor name patch tests/server/services/agent_success_evaluation/test_group_evaluation_runner_regen.py	test_force_regenerate_deletes_prior_results patches get_extractor_name to "overall_success" and tightens assertions to verify by-id deletion with captured IDs. Both test_regenerate_happy_path_with_real_sqlite_storage and test_regenerate_failure_preserves_old_rows_with_real_sqlite_storage add the same extractor-name patch for evaluation-naming alignment.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A timestamp once wandered too far from the track,
So I tucked the safe limits right into the pack.
No more inline clamping spread hither and yon—
One helper to rule them, the worries are gone.
The regen tests follow with mock IDs aligned,
A tidy refactor, for one tidy mind! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the two main changes: clamping _epoch_to_iso to representable range and repairing regen tests to align with PR #168.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nightly/check-and-test-epoch-clamp

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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
`@tests/server/services/agent_success_evaluation/test_group_evaluation_runner_regen.py`:
- Around line 132-137: The mock for
storage.get_agent_success_evaluation_result_ids currently uses a static
return_value that returns all seeded IDs regardless of input arguments, which
doesn't match the actual storage layer filtering behavior based on session_id,
evaluation_name, and agent_version. Replace the static return_value with a
side_effect that defines a function to filter and return only the matching
result IDs from prior_results based on the provided arguments, ensuring the test
validates that the runner passes the correct arguments to this method.

🪄 Autofix (Beta)

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: Pro Plus

Run ID: e784cbb0-6b37-47b8-8794-3ef363ea6f25

📥 Commits

Reviewing files that changed from the base of the PR and between 15b4777 and c077b71.

📒 Files selected for processing (3)

• reflexio/server/services/storage/sqlite_storage/_base.py
• reflexio/server/services/storage/sqlite_storage/_shadow_verdicts.py
• tests/server/services/agent_success_evaluation/test_group_evaluation_runner_regen.py

[yilu331]

Re: the Docstring Coverage 77.78% pre-merge warning — the undocumented functions in _base.py (the _row_to_* converters) are pre-existing and untouched by this PR. This change adds a documented helper (_result_ids_for, Google-style docstring per the repo rule), which raises coverage rather than lowering it. I have intentionally not padded the unrelated pre-existing converters with docstrings just to clear the threshold, since that would be noise in an otherwise small fix. Happy to do a separate docstring sweep of _base.py if you want the module brought up to 80%+.