Fix stream id counter

[ccamel]

Minor but important changes:

• Reset stream ID counter in VM.ResetEnv() to fully clear state between runs.
• Drop atomic ops on the stream ID counter to match the single-threaded stream handling.

[coderabbitai]

Walkthrough

This change removes atomic synchronization from the stream ID counter implementation and makes the reset function private. The counter now increments non-atomically, the exported ResetStreamIDCounter function is deleted and replaced with a private resetStreamIDCounter, and the sync/atomic import is removed. The reset is now called from ResetEnv.

Changes

Cohort / File(s)	Summary
Stream ID counter simplification engine/stream.go	Removed atomic operations from nextStreamID counter; deleted exported ResetStreamIDCounter and added private resetStreamIDCounter; removed sync/atomic import.
Test updates engine/builtin_test.go, engine/stream_test.go	Updated function calls from ResetStreamIDCounter() to resetStreamIDCounter() across test code.
VM environment integration engine/vm.go	Added resetStreamIDCounter() call at the start of ResetEnv.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Straightforward refactoring across 4 files with consistent pattern (removing atomic operations, renaming and privatizing a function)
• Minimal logic changes; primarily API simplification and integration

Possibly related PRs

• Make stream representation deterministic #39: Directly related — modifies the same stream ID counter implementation, with this PR removing the atomic-backed counter and exported reset that the referenced PR originally introduced.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix stream id counter' accurately reflects the main objectives of the changeset: fixing the stream ID counter by resetting it and removing atomic operations.
Description check	✅ Passed	The description directly addresses the changeset, detailing the two key changes: resetting the stream ID counter in VM.ResetEnv() and removing atomic operations for single-threaded handling.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/stream_id_counter

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7069de and 6f274c2.

📒 Files selected for processing (4)

• engine/builtin_test.go (1 hunks)
• engine/stream.go (1 hunks)
• engine/stream_test.go (5 hunks)
• engine/vm.go (1 hunks)

🔇 Additional comments (5)

engine/stream.go (2)

28-31: LGTM!

Making this function private is appropriate since it's only used internally by the VM's ResetEnv() method and tests within the same package.

---

23-25: The review concern about race conditions is not applicable to this codebase. Stream constructors are called only in sequential, single-threaded contexts and never from goroutines. The single-threaded assumption stated in the PR description is correct and supported by actual usage patterns throughout the codebase.

Likely an incorrect or invalid review comment.

engine/stream_test.go (1)

17-17: LGTM!

The test updates correctly reflect the renamed private function resetStreamIDCounter(). All test logic remains unchanged.

Also applies to: 29-29, 41-41, 53-53, 65-65

engine/vm.go (1)

396-396: Good addition.

Resetting the stream ID counter in ResetEnv() is consistent with resetting other global state. ResetEnv() is only called during interpreter initialization (in NewEmpty() and New()), before any streams are created or added to the VM, so no cleanup of existing streams is needed.

engine/builtin_test.go (1)

4455-4455: Updated helper call to private resetStreamIDCounter is appropriate

Using the unexported resetStreamIDCounter() here keeps the test aligned with the new stream counter API while still ensuring deterministic stream IDs for TestPutChar. No further changes needed.

---

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.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@           Coverage Diff           @@
##             main      #40   +/-   ##
=======================================
  Coverage   98.27%   98.27%           
=======================================
  Files          24       24           
  Lines        7053     7055    +2     
=======================================
+ Hits         6931     6933    +2     
  Misses         90       90           
  Partials       32       32           

Files with missing lines	Coverage Δ
engine/stream.go	100.00% <100.00%> (ø)
engine/vm.go	95.63% <100.00%> (+0.01%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.