Skip to content

test: wait on a deadline in actor recovery reconcile check - #181

Merged
senamakel merged 1 commit into
mainfrom
fix/flaky-actor-recovery-wait
Oct 26, 2026
Merged

senamakel merged 1 commit into
mainfrom
fix/flaky-actor-recovery-wait

Conversation

@senamakel

@senamakel senamakel commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

main CI after #180 failed once in aborting_an_admitted_actor_reconciles_its_running_row: assert_reconciled gave the background cleanup only 128 yield_now turns, which a loaded runner can exhaust. It now polls with a 10s wall-clock deadline. The same test passed on both PR runs and in the release run, which marks it a scheduling flake rather than a regression from the dependency bump.

Summary by CodeRabbit

  • Tests
    • Recovery checks now wait for durable cleanup to settle before running assertions, polling at short intervals for up to ten seconds. This replaces a fixed number of yield-only attempts and makes test timing more consistent across different runtime conditions.

The reconciliation helper polled a fixed number of yields, which a loaded CI runner could exhaust before cleanup tasks were scheduled. It now polls against a ten second deadline with a short sleep between attempts, so the assertion reflects whether recovery actually completed rather than how quickly the scheduler ran.

Auto-committed-on: cuttlefish
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@tinysweeper

tinysweeper Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: bfccd0bf041d8ccddea8a742f285a2dede0a4bdb

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 1 of 1 files.
Pull request purpose (title): test: wait on a dead…

Diagnostics and supporting details

Full summary

Pull request purpose (title): test: wait on a deadline in actor recovery reconcile check. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface Files Review signal Count
Production 0 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

Features

  • Modified — Deadline-based polling in `assert_reconciled` test helper: Replaces the bounded 128-yield budget with a 10-second wall-clock deadline and 5 ms sleeps, so background cleanup can make progress on a loaded CI runner; the assertion that actor recovery finishes durable cleanup is not weakened. (crates/tinyflows-module/src/run_manager_actor_recovery_tests.rs async fn assert_reconciled()

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The critique lane found no defect worth blocking on, noting the wall-clock deadline is a sane way to make the wait robust on a loaded CI runner.
  • Lane summary: The change replaces a fixed 128-iteration yield loop in the test helper `assert_reconciled` with a wall-clock deadline plus a 5 ms sleep, which is a sane way to make the wait robust on a loaded CI runner. The tests using this helper are not started with paused time, so the real-time deadline and sleeps advance normally, and the file stays well under the 500-line limit. I found no defect worth blocking on. _The code index is behind this pull request (indexed at `5ca41529df89`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane found no defects, observing the change touches only test code with no untrusted-input path, network/subprocess call, or permission change.
  • Lane summary: The change replaces a fixed 128-yield polling budget with a 10-second wall-clock deadline plus 5 ms sleeps in a test helper that waits for actor-recovery cleanup. It only touches test code, adds no untrusted-input path, network/subprocess call, or permission change, and stays within the 500-line file limit. No defects found. _The code index is behind this pull request (indexed at `5ca41529df89`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The only change is to a test helper for actor recovery: a fixed 128-yield budget becomes a 10-second wall-clock deadline with 5ms sleeps before the final `expect`. This makes the helper strictly more forgiving (more polling iterations, real sleeping so background cleanup tasks can make progress) and does not weaken any assertion — `settled.expect(...)` and the subsequent status assertions are untouched. No production code changed, so no test coverage is owed; the change looks sound. _The code index is behind this pull request (indexed at `5ca41529df89`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane reported no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `5ca41529df89`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The diff replaces a fixed 128-yield budget with a 10-second wall-clock deadline polling every 5 ms in the test-only `assert_reconciled` helper, which is exactly what the description claims and stays inside the 500-line file limit. The change matches its description and looks safe to merge. _The code index is behind this pull request (indexed at `5ca41529df89`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000146
  • Tokens: 88697 input · 2105 output · 11776 cached · 67 embedding
  • Updated: 2026-10-26T07:34:38Z
Head State Pass summary
bfccd0bf041d ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-26T07:34:38Z)

tinysweeper 0.1.2

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T07:35:05.816486Z bfccd0b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0dfd9f92-7d85-4930-9df5-fba7663a9619
Commits

Reviewing files that changed from the base of the PR and between 3177b47 and bfccd0b.

Files selected for processing: 1
  • crates/tinyflows-module/src/run_manager_actor_recovery_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The recovery test helper now polls until a 10-second deadline and waits 5 ms between checks. Assertions still run after durable cleanup settles.

Changes: Recovery test polling

Layer / File(s) Summary
Bounded reconciliation polling
crates/tinyflows-module/src/run_manager_actor_recovery_tests.rs
assert_reconciled polls until a 10-second deadline and waits 5 ms between checks.

Priority: Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Other

Suggested reviewers: m3ga-mind

Merge Risk: Minimal · up to bfccd

The recovery test now waits for cleanup with a bounded polling deadline and has no identified merge-blocking risk.

Pre-merge checks: Passed 4 · Failed 1

Failed 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "test: wait on a deadline in actor recovery reconcile check" matches the main change. The raw summary and PR objectives both say the recovery test helper replaces a fixed count of 128 yield_…
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.
Full details: Title check

Explanation

The title "test: wait on a deadline in actor recovery reconcile check" matches the main change. The raw summary and PR objectives both say the recovery test helper replaces a fixed count of 128 yield_now iterations with polling against a 10-second wall-clock deadline, with a 5 ms sleep between checks. The title names the test type, the deadline-based wait, and the reconcile check. It is specific and accurate, and it does not include file lists or vague wording.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit waits beside the test,
Five milliseconds, then checks the nest.
Cleanup settles, calm and clear,
The deadline keeps the bounds in gear.
Then assertions hop along.

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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0001

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 26, 2026
@senamakel
senamakel merged commit 128d0f3 into main Oct 26, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant