feat: test environment mode for historical pathfinding#16
Open
leinss wants to merge 5 commits into
Open
Conversation
Adds a "Test Environment" toggle to the path builder form that enables querying the pathfinder against historical blockchain state at a specific block number. - testEnvService.js: session management (create, destroy, health check) - circlesApi.js: findPathViaTestEnv sends circlesV2_findPath through test-env RPC proxy (which adds X-Max-Block-Number header) - PathFinderForm: test-env toggle, URL input, block number input, session status indicator - FlowVisualization: auto-creates session on first query, reuses for subsequent queries, shows warnings on error The test-env mode is mutually exclusive with staging mode. Session auto-extends TTL on each query and expires after 30m idle.
- Add missing simulatedConsentedAvatars to findPathViaTestEnv - Add BigInt-safe response handling (stringifyBigInts on RPC result) - Validate targetFlow with String(BigInt()) before sending - Add error logging (console.error) in findPathViaTestEnv - Remove dead code: sessionTimer, sendRpcRequest - Consolidate DEFAULT_TEST_ENV_URL to single source (circlesApi.js) - Explicitly construct session object instead of spreading server response - Validate sessionId exists before constructing rpcProxyUrl - Add resolveBaseUrl helper to eliminate URL normalization duplication
- Guard against undefined rpcResponse.result in findPathViaTestEnv - Log warning on session destroy failure (was silently swallowed)
Summary by CodeRabbit
WalkthroughTest-environment mode added to pathfinding: new form fields and handlers let users enable a test RPC proxy and specify URL/block. A test-env session is created/reused via a new service, session lifecycle is managed, and pathfinding calls route through the session's RPC proxy when enabled. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/FlowVisualization.jsx`:
- Around line 427-435: The code only checks that baseData.TestEnvBlockNumber is
non-empty but then calls getOrCreateSession(Number(...)) without validating
format; update the FlowVisualization.jsx logic before calling getOrCreateSession
to validate baseData.TestEnvBlockNumber is a whole non-negative integer (e.g.,
parse with parseInt/Number, ensure Number.isInteger and value >= 0 and not NaN),
and if invalid call setFormWarnings(['Enter a valid non-negative integer block
number for test environment mode']) and return; keep existing call to
getOrCreateSession with the validated numeric block number.
- Around line 425-443: The test-env session logic is only applied in
handleFindPath, so other callers of executeFindPath (e.g., quick-filter
auto-search) omit testEnvSession and bypass test mode; move the TestEnv handling
into executeFindPath itself (or add a small helper called by every caller) so
executeFindPath will: if baseData.UseTestEnv is true, validate
TestEnvBlockNumber, call getOrCreateSession(baseData.TestEnvUrl,
Number(baseData.TestEnvBlockNumber)), call setTestEnvSession(session) on
success, and include testEnvSession on the request payload; on error call
setFormWarnings([...]) and setTestEnvSession(null). Update executeFindPath
callers to stop duplicating this logic if present.
In `@src/services/circlesApi.js`:
- Around line 115-117: When formData.UseTestEnv is true but
formData.testEnvSession is missing, do not fall through to the live/staging RPC;
instead detect this condition near the existing check that calls
findPathViaTestEnv(formData) and explicitly fail (throw an Error or return a
rejected Promise) with a clear message (e.g., "testEnvSession required when
UseTestEnv is true") so callers cannot silently get live data; update the branch
that currently returns findPathViaTestEnv to first validate testEnvSession and
only call findPathViaTestEnv when present, otherwise throw/return the explicit
failure.
In `@src/services/testEnvService.js`:
- Around line 21-23: The code converts blockNumber with Number(blockNumber) into
numBlock without validation, allowing NaN, decimals or negatives; update the
session-creation logic (where const numBlock = Number(blockNumber)) to validate
blockNumber first: parse/convert to an integer, ensure it's a finite integer (no
decimals), >= 0, and not NaN, and if invalid return/throw a clear client-side
error before calling resolveBaseUrl/test-env session creation; apply the same
validation to the other occurrence around lines 41-44 so both code paths reject
invalid block numbers early.
- Around line 81-85: The DELETE fetch call to
`${baseUrl}/api/v1/session/${sessionId}` currently ignores non-2xx responses;
change it to capture the response (const res = await fetch(...)), check res.ok,
and when false log a failure including res.status, res.statusText and response
body (await res.text() or JSON) for debugging and then throw or handle as an
error so it flows into the existing catch; update the log message that
references sessionId to include those response details.
🪄 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: CHILL
Plan: Pro
Run ID: 97ab9f73-afeb-424c-b258-bfdfa476660a
📒 Files selected for processing (6)
src/components/CollapsibleLeftPanel.jsxsrc/components/FlowVisualization.jsxsrc/components/PathFinderForm.jsxsrc/hooks/useFormData.jssrc/services/circlesApi.jssrc/services/testEnvService.js
Comment on lines
+21
to
+23
| const baseUrl = resolveBaseUrl(testEnvUrl); | ||
| const numBlock = Number(blockNumber); | ||
|
|
There was a problem hiding this comment.
Validate block number before creating a test-env session.
Line 22 accepts any Number(...) value. Invalid values (NaN, decimal, negative) should be rejected client-side before sending the session-create request.
Suggested fix
export async function getOrCreateSession(testEnvUrl, blockNumber) {
const baseUrl = resolveBaseUrl(testEnvUrl);
const numBlock = Number(blockNumber);
+ if (!Number.isInteger(numBlock) || numBlock < 0) {
+ throw new Error('Block number must be a non-negative integer');
+ }Also applies to: 41-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/testEnvService.js` around lines 21 - 23, The code converts
blockNumber with Number(blockNumber) into numBlock without validation, allowing
NaN, decimals or negatives; update the session-creation logic (where const
numBlock = Number(blockNumber)) to validate blockNumber first: parse/convert to
an integer, ensure it's a finite integer (no decimals), >= 0, and not NaN, and
if invalid return/throw a clear client-side error before calling
resolveBaseUrl/test-env session creation; apply the same validation to the other
occurrence around lines 41-44 so both code paths reject invalid block numbers
early.
Comment on lines
+81
to
+85
| await fetch(`${baseUrl}/api/v1/session/${sessionId}`, { method: 'DELETE' }); | ||
| } catch (err) { | ||
| // Non-fatal — session will expire via TTL, but log for debugging | ||
| console.warn(`Failed to destroy test-env session ${sessionId}:`, err.message); | ||
| } |
There was a problem hiding this comment.
Log non-2xx session-destroy responses as failures.
At Line 81-85, HTTP error statuses are not treated as failures, so server-side destroy errors can pass silently.
Suggested fix
try {
- await fetch(`${baseUrl}/api/v1/session/${sessionId}`, { method: 'DELETE' });
+ const response = await fetch(`${baseUrl}/api/v1/session/${sessionId}`, { method: 'DELETE' });
+ if (!response.ok) {
+ const detail = await response.text().catch(() => '');
+ console.warn(`Failed to destroy test-env session ${sessionId}: HTTP ${response.status}${detail ? ` — ${detail}` : ''}`);
+ }
} catch (err) {
// Non-fatal — session will expire via TTL, but log for debugging
console.warn(`Failed to destroy test-env session ${sessionId}:`, err.message);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/testEnvService.js` around lines 81 - 85, The DELETE fetch call
to `${baseUrl}/api/v1/session/${sessionId}` currently ignores non-2xx responses;
change it to capture the response (const res = await fetch(...)), check res.ok,
and when false log a failure including res.status, res.statusText and response
body (await res.text() or JSON) for debugging and then throw or handle as an
error so it flows into the existing catch; update the log message that
references sessionId to include those response details.
… routing - Move test-env session management from handleFindPath into executeFindPath so all callers (quick-filter, auto-search) correctly route through test-env - Validate block number as non-negative integer before session creation - Throw explicitly when UseTestEnv is true but no session attached (prevent silent fallback to live RPC) - Log non-2xx responses on session DELETE for debugging
- Only show Test Environment toggle when Use Staging is enabled - Auto-enable staging when test-env is toggled on - Normalize persisted form state (test-env implies staging) - Add Close button to manually destroy session and free a slot - Show TTL/max-sessions info when no session is active - Surface max-sessions-reached error with actionable hint - Muted session indicator styling (gray instead of bright green)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/FlowVisualization.jsx`:
- Around line 361-372: The current try/catch wraps both getOrCreateSession and
loadPathData so any loadPathData failure is misreported as a session error;
change it to try/catch only around getOrCreateSession (call getOrCreateSession,
setTestEnvSession(session) inside that try) and keep the existing error handling
with the isMaxSessions hint and setTestEnvSession(null) only in that catch; then
call await loadPathData({...requestData, testEnvSession: session}) outside that
catch so its errors propagate through the normal request error path (do not
clear setTestEnvSession on loadPathData failures).
- Around line 376-377: The code currently only clears the local reference via
setTestEnvSession(null) and then calls loadPathData(requestData), leaving the
remote test session alive; update the logic to first check the existing test
session (the state referenced by setTestEnvSession/testEnvSession) and call its
shutdown method (e.g., destroy/close/terminate API on that session) and await
its completion, handling errors, before nulling it and calling loadPathData;
also add the same teardown when toggling UseTestEnv/UseStaging off in
PathFinderForm (watch the toggles and destroy the live session on exit) so no
orphaned server sessions remain.
🪄 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: CHILL
Plan: Pro
Run ID: 817218f3-fcfa-49cb-a096-ba12759d09d2
📒 Files selected for processing (6)
src/components/CollapsibleLeftPanel.jsxsrc/components/FlowVisualization.jsxsrc/components/PathFinderForm.jsxsrc/hooks/useFormData.jssrc/services/circlesApi.jssrc/services/testEnvService.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/PathFinderForm.jsx
- src/services/circlesApi.js
- src/services/testEnvService.js
Comment on lines
+361
to
+372
| try { | ||
| const session = await getOrCreateSession(requestData.TestEnvUrl, parsedBlock); | ||
| setTestEnvSession(session); | ||
| await loadPathData({ ...requestData, testEnvSession: session }); | ||
| } catch (err) { | ||
| const isMaxSessions = err.message.includes('Maximum concurrent sessions'); | ||
| const hint = isMaxSessions | ||
| ? ' — close unused sessions or wait for them to expire (30 min TTL).' | ||
| : ''; | ||
| setFormWarnings([`Test-env session error: ${err.message}${hint}`]); | ||
| setTestEnvSession(null); | ||
| } |
There was a problem hiding this comment.
Separate session creation failures from query failures.
loadPathData(...) is inside the same try as getOrCreateSession(...), so any downstream RPC/pathfinder failure is reported as a test-env session error and testEnvSession is cleared even though the session was created successfully. Catch only the session acquisition step here and let query failures surface through the normal request error path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/FlowVisualization.jsx` around lines 361 - 372, The current
try/catch wraps both getOrCreateSession and loadPathData so any loadPathData
failure is misreported as a session error; change it to try/catch only around
getOrCreateSession (call getOrCreateSession, setTestEnvSession(session) inside
that try) and keep the existing error handling with the isMaxSessions hint and
setTestEnvSession(null) only in that catch; then call await
loadPathData({...requestData, testEnvSession: session}) outside that catch so
its errors propagate through the normal request error path (do not clear
setTestEnvSession on loadPathData failures).
Comment on lines
+376
to
377
| setTestEnvSession(null); | ||
| await loadPathData(requestData); |
There was a problem hiding this comment.
Destroy the live session when leaving test-env mode.
This branch only clears the local reference. Because PathFinderForm hides the session UI as soon as UseTestEnv is off, any still-live server session becomes unreachable and continues consuming one of the limited session slots until TTL expiry. Tear the session down before hiding the mode, or watch UseTestEnv/UseStaging and destroy on exit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/FlowVisualization.jsx` around lines 376 - 377, The code
currently only clears the local reference via setTestEnvSession(null) and then
calls loadPathData(requestData), leaving the remote test session alive; update
the logic to first check the existing test session (the state referenced by
setTestEnvSession/testEnvSession) and call its shutdown method (e.g.,
destroy/close/terminate API on that session) and await its completion, handling
errors, before nulling it and calling loadPathData; also add the same teardown
when toggling UseTestEnv/UseStaging off in PathFinderForm (watch the toggles and
destroy the live session on exit) so no orphaned server sessions remain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
circlesV2_findPathcalls through the test-env RPC proxy (which addsX-Max-Block-Numberheader)New files
testEnvService.js— session lifecycle management (create, destroy, health, block check)Key details
stringifyBigIntsFlowRequestparameters supported includingsimulatedConsentedAvatarsDEFAULT_TEST_ENV_URLKnown v1 limitations (documented in code)
Related PRs
Test plan