Skip to content

Time-of-day flake: test_update_routine_cron_expr_recomputes_next_fire reds every CI run in the 02:55-03:00Z window - #2370

Merged
jaylfc merged 1 commit into
devfrom
exec/tsk-kboqzh
Aug 12, 2026
Merged

Time-of-day flake: test_update_routine_cron_expr_recomputes_next_fire reds every CI run in the 02:55-03:00Z window#2370
jaylfc merged 1 commit into
devfrom
exec/tsk-kboqzh

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 12, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Time-of-day flake: test_update_routine_cron_expr_recomputes_next_fire reds every CI run in the 02:55-03:00Z window

Autonomous build of board card tsk-kboqzh.

  • add _clock seam to RoutineStore (defaults to time.time)
  • use frozen clock in test to avoid 02:55-03:00Z collision window

Files:
tests/projects/test_routines_store.py | 1 +
tinyagentos/projects/routines_store.py | 7 ++++---
2 files changed, 5 insertions(+), 3 deletions(-)

Summary by CodeRabbit

  • Bug Fixes
    • Improved routine scheduling consistency by ensuring timestamps and schedule calculations use a consistent clock.
    • Fixed timing-sensitive behavior that could produce inconsistent cron updates.

Red proof at merge base (card demand: 02:57:30Z window)

Clock frozen at 2026-08-12T02:57:30Z (epoch 1786503450.0) via a pytest plugin monkeypatching time.time; run at merge base 7d38483e (pre-fix):

$ PYTHONPATH=. pytest tests/projects/test_routines_store.py::test_update_routine_cron_expr_recomputes_next_fire -p fake_time_plugin
>       assert updated["next_fire"] != old_next
E       assert 1786503600.0 != 1786503600.0
tests/projects/test_routines_store.py:221: AssertionError
FAILED tests/projects/test_routines_store.py::test_update_routine_cron_expr_recomputes_next_fire
1 failed in 0.55s

Both 0 3 * * * and */5 * * * * compute next_fire 1786503600.0 (03:00:00Z) inside the 02:55-03:00Z window, exactly the carded flake. Same frozen-clock run at PR head b2be024e: 27 passed in the full file — which also proves the _clock seam is live, since an inert seam would fail identically to base.

- add _clock seam to RoutineStore (defaults to time.time)
- use frozen clock in test to avoid 02:55-03:00Z collision window
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

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: 2e835d32-e02c-4327-900e-78ac7dd716a3

📥 Commits

Reviewing files that changed from the base of the PR and between 7d38483 and b2be024.

📒 Files selected for processing (2)
  • tests/projects/test_routines_store.py
  • tinyagentos/projects/routines_store.py

📝 Walkthrough

Walkthrough

RoutineStore now uses an overrideable class-level clock for routine creation, updates, and schedule calculations. The cron rescheduling test sets a fixed clock value to make its timestamps deterministic.

Changes

Routine clock control

Layer / File(s) Summary
Clock injection and deterministic scheduling
tinyagentos/projects/routines_store.py, tests/projects/test_routines_store.py
RoutineStore uses _clock() for creation timestamps, update timestamps, and next_fire calculations. The cron rescheduling test sets a fixed clock value.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the flaky routine cron test and the specific time window that causes CI failures.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch exec/tsk-kboqzh

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

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar


@pytest.mark.asyncio
async def test_update_routine_cron_expr_recomputes_next_fire(store):
store._clock = staticmethod(lambda: 43200.0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: store._clock = staticmethod(lambda: 43200.0) stores a staticmethod descriptor object as an instance attribute. When create_routine later calls self._clock(), CPython's instance-attribute lookup returns the raw __dict__ value without invoking the descriptor protocol (__get__), so the call receives the staticmethod wrapper itself — which is not callable — and raises TypeError: 'staticmethod' object is not callable.

The class-level _clock = staticmethod(time.time) is correct only when the class attribute itself is accessed (descriptor __get__ fires, returning the inner function). The test's instance-level assignment bypasses that mechanism entirely, so this test will fail at runtime.

Store a plain callable instead:

Suggested change
store._clock = staticmethod(lambda: 43200.0)
store._clock = lambda: 43200.0

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Refuted on this repo's supported Pythons: staticmethod objects implement __call__ since Python 3.10 (bpo-43682), and requires-python here is >=3.11. The instance-attribute lookup does return the wrapper, but calling the wrapper directly works and forwards to the inner callable. Proven empirically: I ran the full file at head with wall-clock frozen to 02:57:30Z and got 27 passed - only possible if the instance override was exercised (the pre-fix base reds under the same frozen clock; fenced output in the PR body). A bare lambda would read cleaner, but there is no runtime defect; not worth a CI cycle. Resolving.

@kilo-code-bot

kilo-code-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 0
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
tests/projects/test_routines_store.py 215 store._clock = staticmethod(lambda: 43200.0) stores a staticmethod descriptor object as an instance attribute. CPython's instance-dict lookup returns the raw value without invoking __get__, so self._clock() later receives the non-callable staticmethod wrapper and raises TypeError. Use a plain callable: store._clock = lambda: 43200.0
Files Reviewed (2 files)
  • tests/projects/test_routines_store.py - 1 issue
  • tinyagentos/projects/routines_store.py - no issues

Fix these issues in Kilo Cloud


Reviewed by step-3.7-flash · Input: 76K · Output: 7.3K · Cached: 177.3K

@jaylfc

jaylfc commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

nemotron-super review

VERDICT: No blocking issues found.

Automated first-pass review by the nemotron-super lane. The lead still reviews before merge.

@jaylfc
jaylfc merged commit 2eb1a8f into dev Aug 12, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant