test kvs short-circuit and store.set revert paths in LibFlow.flow

[thedavidmeister]

Summary

Two new tests pinning LibFlow.flow's store-side behavior:

• testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty — explicit count=0 expectCall on STORE.set when kvs is empty. Locks the existing short-circuit so a future change that drops it is caught. Mutation verified: removing the kvs.length > 0 guard makes the test fail; reverting passes.

• testFlowBasicFlowTimeStoreSetRevertBubbles — when kvs is non-empty and store.set reverts, the revert MUST propagate out of flow() rather than be caught and swallowed.

Closes #329.

Test plan

• both tests pass on current code (one as static, one as 100-run fuzz)
• mutation: drop the kvs.length > 0 guard → empty-kvs test fails

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Enhanced test coverage for storage operations, including validation of empty input scenarios and error propagation handling.

[coderabbitai]

Warning

Rate limit exceeded

@thedavidmeister has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 56 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 65f265dc-b97f-4a5f-a374-d6956aacfac0

📥 Commits

Reviewing files that changed from the base of the PR and between c8746fb and c1d6de0.

📒 Files selected for processing (1)

• test/src/concrete/Flow.time.t.sol

Walkthrough

Two new test cases verify uncovered code paths in LibFlow.flow: one asserts interpreterStore.set is not called when kvs is empty; another confirms that reverts from interpreterStore.set propagate correctly out of flow.flow.

Changes

LibFlow.flow edge-case test coverage

Layer / File(s)	Summary
Test: Empty kvs branch test/src/concrete/Flow.time.t.sol	testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty mocks STORE.set and asserts via vm.expectCall(..., 0) that it is never invoked when kvs is empty.
Test: Store revert propagation test/src/concrete/Flow.time.t.sol	testFlowBasicFlowTimeStoreSetRevertBubbles mocks STORE.set to revert with a custom payload and asserts the revert bubbles out of flow.flow via vm.expectRevert.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• [TEST-4] [LOW] testFlowBasicFlowTime tests store writes, not time #422: Related to interpreterStore.set behavior and write coverage in Flow.time.t.sol; this PR adds the explicit edge-case assertions for empty kvs and revert propagation that may address overlapping testing objectives.

🚥 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 main change: adding tests for the kvs short-circuit and store.set revert paths in LibFlow.flow, which matches the two new test functions added.
Linked Issues check	✅ Passed	Both tests from issue #329 are implemented: testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty verifies empty kvs doesn't call set, and testFlowBasicFlowTimeStoreSetRevertBubbles verifies store.set reverts propagate correctly.
Out of Scope Changes check	✅ Passed	All changes are within scope: two test functions added to Flow.time.t.sol directly address the objectives in issue #329 with no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2026-05-05-issue-329-kvs-shortcircuit-tests

---

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 `@test/src/concrete/Flow.time.t.sol`:
- Around line 59-63: Replace the ABI-encoded revert payloads with raw string
literals: in the vm.mockCallRevert call that currently passes
abi.encode("STORE_SET_FAILED") (with IInterpreterStoreV2.set.selector) and in
the vm.expectRevert call that uses abi.encode("STORE_SET_FAILED"), change both
to the raw string "STORE_SET_FAILED" so the mock revert and the expected revert
use the same raw UTF-8 message per Foundry convention.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8cd9ee35-495a-4e6e-ade6-3a31d9623c5b

📥 Commits

Reviewing files that changed from the base of the PR and between ceaea3c and c8746fb.

📒 Files selected for processing (1)

• test/src/concrete/Flow.time.t.sol