fix: Cleanup FallThrough sessions

[grahamking]

• Automatically when they finish (session_final).
• After one hour of inactivity, to catch those that never finish
  cleanly.

Assisted-by: Codex:GPT 5.6 Sol medium
Signed-off-by: Graham King grahamk@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Improved session lifecycle management to remove completed and stale sessions automatically.
  • Preserved active or recently accessed sessions during cleanup.
  • Ensured session cleanup occurs even when execution encounters errors.
  • Removed session eviction data after completion to prevent lingering state.

• Tests

  • Added coverage for final-session cleanup, activity timestamp updates, and inactive-session removal.

[grahamking]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://NVIDIA-NeMo.github.io/Switchyard/pr-preview/pr-313/
Built to branch gh-pages at 2026-08-05 20:49 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[coderabbitai]

Walkthrough

The router now stores shared session state with access timestamps, starts periodic weak-reference cleanup, removes completed sessions after success or errors, and clears session eviction records. Tests cover final-session cleanup, access refresh, and stale-session removal.

Changes

Session lifecycle management

Layer / File(s)	Summary
Timestamped session registry crates/libsy/src/algorithms/fall_through.rs	Session records now contain shared state and last_accessed timestamps. Lookups refresh timestamps. Stateful and stateless routers initialize shared registries and cleanup guards.
Execution and completion cleanup crates/libsy/src/algorithms/fall_through.rs, crates/libsy/src/core/algorithm.rs	Execution extracts the session once. Final sessions are removed after routing success or failure. Completion removes routing state and session eviction records.
Periodic idle-session cleanup crates/libsy/src/algorithms/fall_through.rs	A periodic cleanup task removes stale idle sessions while retaining active or recently accessed sessions. Tests validate final-session removal, timestamp refresh, and cleanup behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Poem

I’m a rabbit who guards every session at night,
Refreshing old timestamps till the state feels right.
Stale paths hop away, completed trails close,
Eviction records vanish like dew on a rose.
Tests thump their paws: “The cleanup is sound!”
And fresh session state springs up from the ground.

🚥 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 clearly and concisely describes the primary change: automatic cleanup of FallThrough sessions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

---

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

crates/libsy/src/algorithms/fall_through.rs (1)

366-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the cleanup task's cadence and exit condition.

The task's lifecycle is not obvious from the signature: the first tick is delayed by one interval, missed ticks are skipped, and the loop ends when the router is dropped. The guidelines require comments for async and lifecycle behavior.

♻️ Proposed doc comment

+/// Sweeps idle session state once per [`SESSION_CLEANUP_INTERVAL`], starting one
+/// interval after spawn. Holds only a weak handle, so the task ends at the first
+/// tick after the router is dropped.
 async fn cleanup_inactive_sessions<S>(states: Weak<SessionStates<S>>)

🤖 Prompt for 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.

In `@crates/libsy/src/algorithms/fall_through.rs` around lines 366 - 372, Document
the lifecycle behavior in cleanup_inactive_sessions: state that the first tick
is delayed by SESSION_CLEANUP_INTERVAL, missed ticks are skipped, and the
cleanup loop exits when the router/session state is dropped. Place the comment
near the interval setup or loop, without changing the existing timing or
termination behavior.

Source: Coding guidelines

🤖 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 `@crates/libsy/src/algorithms/fall_through.rs`:
- Around line 42-43: Update SESSION_STATE_TTL to one hour so unfinished sessions
are evicted after the intended inactivity period, while keeping
SESSION_CLEANUP_INTERVAL at one hour. Add concise comments above both constants
documenting the cleanup interval and idle-state eviction policy.
- Around line 1345-1357: Update the test cleanup setup in
cleanup_removes_only_inactive_idle_sessions to compute now as a forward time
using Instant::now() plus SESSION_STATE_TTL and one second, instead of
subtracting from Instant::now() with checked_sub and expect. Preserve the
inserted last_accessed values as stale relative to the new now, and remove the
underflow-prone timestamp calculation.

---

Nitpick comments:
In `@crates/libsy/src/algorithms/fall_through.rs`:
- Around line 366-372: Document the lifecycle behavior in
cleanup_inactive_sessions: state that the first tick is delayed by
SESSION_CLEANUP_INTERVAL, missed ticks are skipped, and the cleanup loop exits
when the router/session state is dropped. Place the comment near the interval
setup or loop, without changing the existing timing or termination behavior.

🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c2cd87b4-0aa1-45ed-bc09-9ebf6762ff05

📥 Commits

Reviewing files that changed from the base of the PR and between 2a91472 and 34160e0.

📒 Files selected for processing (2)

• crates/libsy/src/algorithms/fall_through.rs
• crates/libsy/src/core/algorithm.rs

[elyasmnvidian]

A cancelled final request can keep its session data, and one test checks a timestamp rather than cleanup behavior. The suggested changes are inline.