Skip to content

feat(simulation): add explicit post-stabilization hook at end of play()#935

Open
yashveer-1 wants to merge 2 commits intoCircuitVerse:mainfrom
yashveer-1:stabilization-hook
Open

feat(simulation): add explicit post-stabilization hook at end of play()#935
yashveer-1 wants to merge 2 commits intoCircuitVerse:mainfrom
yashveer-1:stabilization-hook

Conversation

@yashveer-1
Copy link

@yashveer-1 yashveer-1 commented Feb 13, 2026

Fixes #

Describe the changes you have made in this PR

This PR generally focuses on how to know whether the state is stable or not. Before this PR, there was no way to determine at what point exactly the circuit is stabilized, which means there was not any explicit function that determines the stabilization of the state.


Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

Explain your implementation approach:

Describe in your own words:

  • This is a no-op code that solves the problem of knowing when the current state is stable.
  • I chose this because it adds reusable code that is specific and clear and adds an explicit function to define the end of a particular state, which makes future debugging easier and reliable.
  • Added the hook after the queue is completely drained, all resolve operations are finished, contention checks have completed, and the function is completely stable; this avoids the conflict of logic between the previous code.

Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

Summary by CodeRabbit

  • Chores
    • Added internal stabilization callback infrastructure to the simulator to enable future extensibility of the simulation lifecycle. This introduces a post-tick boundary hook used internally; there are no user-facing behavior changes or API surface modifications in this release.

@netlify
Copy link

netlify bot commented Feb 13, 2026

Deploy Preview for circuitverse ready!

Name Link
🔨 Latest commit 63627c7
🔍 Latest deploy log https://app.netlify.com/projects/circuitverse/deploys/698f350230dc2300083d08dc
😎 Deploy Preview https://deploy-preview-935--circuitverse.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 44 (🔴 down 4 from production)
Accessibility: 73 (no change from production)
Best Practices: 92 (no change from production)
SEO: 82 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

Walkthrough

A new internal hook function onSimulationStabilized(scope) was added to the engine module. It is a no-op JSDoc-documented function intended as a post-tick boundary. After the simulation queue drains and contentions are handled, the hook is invoked only when no contention was detected. The function is internal and not exported.

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an explicit post-stabilization hook at the end of the play() method. It directly relates to the primary objective of the pull request.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/simulator/src/engine.js (1)

443-458: Consider the step-limit overflow path.

The hadContention guard is a good fix for the contention case. However, when stepCount > 1000000 (Line 434), the loop doesn't return/break — the queue may eventually drain and hadContention remains false, so the hook fires after a "Simulation Stack limit exceeded" error. This is pre-existing behavior, but you may want to guard against it too for consistency with the hook's "stabilized state" contract.

Optional: track the overflow case
     let stepCount = 0
     let elem
+    let stepLimitExceeded = false
     while (!simulationArea.simulationQueue.isEmpty()) {
         if (errorDetected) {
             simulationArea.simulationQueue.reset()
             return
         }
         elem = simulationArea.simulationQueue.pop()
 
         elem.resolve()
 
         stepCount++
         if (stepCount > 1000000) {
             // Cyclic or infinite Circuit Detection
             showError(
                 'Simulation Stack limit exceeded: maybe due to cyclic paths or contention'
             )
             forceResetNodesSet(true)
+            stepLimitExceeded = true
+            break
         }
     }

Then update the guard:

-    if (!hadContention) {
+    if (!hadContention && !stepLimitExceeded) {
         onSimulationStabilized(scope);
     }

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/simulator/src/engine.js`:
- Around line 452-454: The onSimulationStabilized hook is being invoked even
after contention errors where forceResetNodesSet(true) is called, which
contradicts its JSDoc; update play() so the hook is only called on a clean
stabilization (e.g., guard the call by checking the contention/error state
instead of unconditional invocation) or modify the call to pass a status flag
(e.g., onSimulationStabilized(scope, { error: true })) so consumers can
distinguish error vs. clean stabilization; also fix the indentation of the
onSimulationStabilized(scope) line to match the rest of play()'s 4-space block
formatting.

@naman79820
Copy link
Contributor

Hey @yashveer-1 can you please add issue number in fixes #

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.

2 participants