[ENG-2304,ENG-2231,ENG-2269,ENG-2279] Separate draft and active session routes #752
Conversation
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
…-2304-sessiondetail-refactor-plan-d-separate-routes-with-tanstack
The constants api.Global and api.Local were renamed in PR #745 to be values of the SlashCommandSource type. This updates the integration tests to use the new constant format. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The ListSessions API was using in-memory session.Info structs that didn't include the EditorState field, even though it was properly stored in the database. This caused draft session content to appear as undefined when fetching sessions. Added EditorState field to session.Info struct and ensured it's populated when converting from database records in both GetSessionInfo and ListSessions methods.
Users reported that newly created draft sessions weren't visible in the drafts list without manually navigating to other views first. Changes: - Added refreshSessions() call when exiting draft launcher via Escape key - Added refreshSessions() call when discarding empty drafts - Auto-refresh sessions list when switching to Drafts view tab - Refactored draft components (DraftLauncher -> DraftLauncherForm) - Fixed routing to use dedicated /sessions/draft route for drafts This ensures the draft list stays synchronized when users create or modify drafts and navigate back to the list.
… draft launcher The draft launcher was showing "Please enter a prompt before launching" even when the editor contained text. This was due to using a stale closure value of the prompt that was computed during render time rather than getting the current editor content at the time of launching. Changed to fetch the prompt directly from the store's responseEditor at the time of validation to ensure we always check the actual current content. Fixes the issue where existing drafts with saved content would incorrectly fail prompt validation when trying to launch with Cmd+Enter.
Previously, launching a draft session with Cmd+Enter didn't automatically navigate to the session due to timing issues with the status monitoring approach. Now navigation happens immediately after successful launch.
…persist selection The model selection was being saved to the database by ModelSelector but wasn't updating the parent component's state. This caused DraftLauncherForm to overwrite the selection with its stale state during auto-sync, resulting in: - StatusBar showing "DEFAULT" after model selection - Model selection not persisting when reopening drafts Fixed by passing full model configuration through callbacks to keep parent state synchronized with database changes.
The proxy configuration fields (proxy_enabled, proxy_base_url, proxy_model_override, proxy_api_key) were stored in the database but not being returned in ListSessions and GetSessionInfo API responses. This caused proxy settings to appear as undefined in the frontend.
…ting drafts Fixed timing issue where localStorage values weren't loaded before component initialization. Now waits for localStorage to be ready before applying saved model configurations. Also removed restrictive condition that prevented Baseten settings from loading.
- Check for actual text content before triggering draft creation - Only create drafts when user provides meaningful input, not on empty editor setup - Add unified draft creation from title, editor, or working directory changes - Optimize with refs and stable callbacks to reduce re-renders - Add comprehensive debug logging to diagnose initialization behavior
Removed @tanstack/react-form, @tanstack/zod-form-adapter, and zod dependencies as we decided not to use TanStack Form for the draft launcher implementation. Also removed the unused draft.ts type definitions file that was created for TanStack Form integration. Additionally disabled ESLint react-hooks/exhaustive-deps rule to reduce false positive warnings during development.
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 50b6678 in 2 minutes and 20 seconds. Click for details.
- Reviewed
2325lines of code in20files - Skipped
0files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:257
- Draft comment:
Using an empty dependency array in handleCreateDraft is acceptable only because refs are used—but add a comment noting that daemonClient is assumed stable. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment points out something important - there is an implicit dependency on daemonClient that isn't tracked in the deps array. This could be a source of bugs if daemonClient ever becomes unstable. However, the comment is purely informational and doesn't suggest any code changes. The rules say not to make purely informative comments. The comment identifies a real potential issue with dependency tracking. But maybe it should suggest actually adding daemonClient to the deps array instead of just documenting the assumption? While adding daemonClient to deps might be better, that would be a different comment. This comment is purely informational and doesn't suggest any actual changes. The comment should be deleted because it is purely informational and doesn't suggest any concrete code changes, which violates our rules about PR comments.
2. humanlayer-wui/src/hooks/useSessionLauncher.ts:164
- Draft comment:
Side effects (localStorage writes) inside setConfig are fine; consider documenting the persistence strategy for clarity. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. humanlayer-wui/src/components/internal/SessionDetail/SessionDetailRouter.tsx:26
- Draft comment:
Ensure session.status uses the SessionStatus enum (e.g. SessionStatus.Draft) consistently rather than 'draft' literal in multiple places. - Reason this comment was not posted:
Comment was on unchanged code.
4. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherInput.tsx:87
- Draft comment:
Consider handling parsing errors more gracefully in initial editor state parsing; e.g. fallback to empty state if JSON parse fails. - Reason this comment was not posted:
Comment was on unchanged code.
5. humanlayer-wui/src/components/internal/SessionDetail/components/ModelSelector.tsx:116
- Draft comment:
Update API key fallback handling: ensure that if an API key isn’t provided, user feedback is consistent (possibly show disabled state). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. humanlayer-wui/src/pages/DraftSessionPage.tsx:27
- Draft comment:
When filtering sessions for drafts, consider using the SessionStatus enum for clarity instead of string literal. - Reason this comment was not posted:
Comment looked like it was already resolved.
7. humanlayer-wui/src/router.tsx:23
- Draft comment:
Route definitions now include the dedicated draft route. Confirm that any legacy references to drafts in Link components are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to confirm that legacy references are updated, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
8. package.json:14
- Draft comment:
Package updates (e.g. @biomejs/biome to ^2.2.6) look correct. Ensure that all workspaces have been tested with the updated version. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is purely informative and asks the PR author to ensure testing, which violates the rules. It doesn't provide a specific suggestion or point out a specific issue.
9. hld/session/manager.go:844
- Draft comment:
Typo note: In the TODO comment, consider using 'JSON-RPC' instead of 'JsonRPC' and 'SQLite' (capitalized) instead of 'sqlite' for consistency with common conventions. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_z6s16HFGDYh9QU9l
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx
Outdated
Show resolved
Hide resolved
…separate-routes-with-tanstack Resolved conflicts in daemon_slash_commands_integration_test.go by accepting main's version using api.SlashCommandSourceGlobal and api.SlashCommandSourceLocal constants instead of string casting.
Removed all DRAFT_DEBUG console.log statements that were added during development. Kept only necessary error logging for actual error cases.
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 4ab151d in 1 minute and 7 seconds. Click for details.
- Reviewed
282lines of code in3files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:103
- Draft comment:
Good cleanup: Removed numerous debug logs to improve clarity. Ensure that essential error logging remains for production diagnostics. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:299
- Draft comment:
Consider adding a cleanup for the debounced sync timer (syncTimerRef) on component unmount to avoid potential memory leaks. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherInput.tsx:78
- Draft comment:
Removed debug logs and standardized error logging (using logger.error) is a positive change. Ensure consistency across error messages. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. humanlayer-wui/src/pages/DraftSessionPage.tsx:38
- Draft comment:
For consistency, consider using a centralized logger (e.g., logger.error) instead of console.error when a draft isn’t found or fails to load. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_YLR0IoOD9Fm2Ixhv
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 8d1de32 in 46 seconds. Click for details.
- Reviewed
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:103
- Draft comment:
Removed an extra newline to clean up the code for better readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_RTDOg0C9F8Lltlvz
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 0dacc5a in 1 minute and 16 seconds. Click for details.
- Reviewed
104lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/Breadcrumbs.tsx:88
- Draft comment:
Consider adding a comment explaining why activeSessionDetail is intentionally omitted from the dependency array in this useEffect. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:559
- Draft comment:
In handleDangerousSkipPermissionsConfirm, setting the dialog open to true then immediately false appears unintended. Verify the intended dialog state update. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:428
- Draft comment:
handleLaunchDraft uses proxyEnabled, proxyBaseUrl, and proxyModelOverride but they are missing from its dependency array. Please add them to avoid stale values. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_HqeoA6FzGSMKaTdd
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 2bab2a7 in 49 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:228
- Draft comment:
You removed the unused 'event' parameter from the handleEditorUpdate callback. This improves clarity. Ensure that no side effects are expected from receiving an event; otherwise, this change is fine. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_6lBZseTfOdWYzowP
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
dexhorthy
approved these changes
Oct 16, 2025
Contributor
Author
|
this appears to have borked slash commands, working on a fix |
…mands - Pass workingDirectoryRef from DraftLauncherForm to DraftLauncherInput - Pass workingDirRef from DraftLauncherInput to ResponseEditor - Use ref in ResponseEditor to avoid closure issue with TipTap extensions - Ensures slash commands receive current working directory value even when session is null - Added debug logging to trace working directory through component chain
…separate-routes-with-tanstack
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 2604113 in 2 minutes and 36 seconds. Click for details.
- Reviewed
2843lines of code in30files - Skipped
0files when reviewing. - Skipped posting
18draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/SessionDetailRouter.tsx:24
- Draft comment:
When a draft session is detected, the component immediately navigates to the dedicated draft route. This clear separation is good. Ensure that upstream code never inadvertently passes a draft session here. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is informative and suggests ensuring that upstream code does not pass a draft session inadvertently. However, it does not provide a specific code suggestion or ask for a specific test to be written. It also does not point out a specific issue with the current code. Therefore, it seems to violate the rule against asking the author to ensure behavior is intended or tested.
2. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:198
- Draft comment:
The lazy draft creation logic using refs (draftCreatingRef and draftCreatedRef) is clever. Verify that race conditions are fully mitigated, especially when the editor triggers multiple updates in rapid succession. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment praises the use of refs for lazy draft creation but then asks the author to verify that race conditions are mitigated. This falls under asking the author to ensure behavior is intended or to double-check things, which is against the rules.
3. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:242
- Draft comment:
The useEffect that loads localStorage values for model/provider persists user configuration for new drafts. Consider adding comments that clarify dependency requirements for future maintainability. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment suggests adding comments for future maintainability, which is a request for purely informative comments. This violates the rule against making purely informative comments.
4. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherInput.tsx:106
- Draft comment:
In the editor change handler, ensure that errors during the asynchronous update to the daemon do not block user input. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. humanlayer-wui/src/components/internal/SessionDetail/components/ModelSelector.tsx:116
- Draft comment:
The useEffect updating provider and model from the session ensures the modal reflects the current session state. Ensure that changes in session properties are correctly handled by the dependency array. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. humanlayer-wui/src/components/internal/SessionDetail/components/ResponseEditor.tsx:484
- Draft comment:
The effectiveWorkingDirRef is correctly used to pass the current working directory to the SlashCommandList. Verify that the useEffect updating the local ref covers all edge cases when the workingDir prop changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that the useEffect hook covers all edge cases, which is against the rules. It does not provide a specific suggestion or point out a specific issue.
7. humanlayer-wui/src/components/internal/SessionDetail/components/StatusBar.tsx:53
- Draft comment:
The StatusBar displays model information by stripping provider prefixes; this is a neat UX touch. Double-check that edge cases (when model is empty) are handled gracefully. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. humanlayer-wui/src/hooks/useSessionLauncher.ts:134
- Draft comment:
The Zustand store for session launcher is well structured with proper localStorage persistence. Review all key names used in localStorage to avoid potential collisions. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. humanlayer-wui/src/lib/daemon/http-client.ts:99
- Draft comment:
The connection logic with retries and timeout via AbortController is robust. Consider making the timeout value configurable instead of hardcoded to 5000ms. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. humanlayer-wui/src/pages/DraftSessionPage.tsx:25
- Draft comment:
Loading an existing draft by fetching from the daemon via listSessions is practical. It might be helpful to handle the case where multiple draft sessions exist more explicitly. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The code is looking for a specific draft by ID, so multiple drafts aren't really relevant here. The comment seems speculative - it's suggesting handling a case without evidence that it's actually a problem. The current implementation correctly finds the draft with matching ID and Draft status. Maybe there are edge cases where multiple drafts could cause issues that I'm not seeing. The comment might be hinting at a real architectural concern. Without concrete evidence of problems with multiple drafts, this is a speculative comment. The code handles the core functionality correctly by finding the specific draft by ID. Delete the comment as it's speculative and suggests handling a case without evidence that it's actually problematic.
11. humanlayer-wui/src/pages/SessionTablePage.tsx:41
- Draft comment:
Refreshing sessions on view mode change to Drafts ensures the list stays current. Verify that this refresh does not introduce UI jank on rapid mode toggling. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that a specific behavior does not introduce UI issues, which violates the rule against asking for verification or confirmation of behavior. It does not provide a specific code suggestion or ask for a specific test to be written.
12. humanlayer-wui/src/router.tsx:22
- Draft comment:
Routing now cleanly separates draft sessions from active sessions via the '/sessions/draft' route; this improves URL semantics and maintainability. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining the change made in the code without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
13. package.json:15
- Draft comment:
The dependency version for @biomejs/biome is updated to '^2.2.6'. Ensure that the caret semver versioning aligns with the release strategy. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is about a dependency version change, which is not allowed according to the rules. It also asks the author to ensure alignment with the release strategy, which is similar to asking for confirmation or verification, which is not allowed.
14. test-slash-commands.md:1
- Draft comment:
The manual test plan is comprehensive and outlines expected behaviors for slash commands in both draft and active sessions. Great work including detailed steps and console checks. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and praises the author for their work on the manual test plan. It does not provide any actionable feedback or suggestions for improvement.
15. humanlayer-wui/src/lib/daemon/types.ts:464
- Draft comment:
The transformSDKSession helper function properly adds default values such as 'dangerouslySkipPermissions'. Consider expanding this to include any new WUI fields as the integration evolves. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
16. humanlayer-wui/src/pages/SessionTablePage.tsx:40
- Draft comment:
Using tabs to toggle between Sessions, Drafts, and Archived in the SessionTablePage improves UX. Verify that the sessionCounts are updated correctly on session creation and updates. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that sessionCounts are updated correctly, which is against the rules. It is not making a specific code suggestion or asking for a specific test to be written. Therefore, this comment should be removed.
17. hld/session/manager.go:844
- Draft comment:
Typographical note: In the TODO comment, consider using 'JSON-RPC' instead of 'JsonRPC' for consistent acronym capitalization. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
18. humanlayer-wui/src/components/DebugPanel.tsx:9
- Draft comment:
Line 9: The import statement starts with a closing brace '}'. This looks like a typographical error. Please review whether a specific component (e.g., Dialog) was meant to be imported from '@/components/ui/dialog' or if the import should be removed/modified. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_X3ZJXQpXTV6xUmqN
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| }, [selectedIndex]) | ||
|
|
||
| // Fetch commands from daemon | ||
| useEffect(() => { |
Contributor
There was a problem hiding this comment.
Good debug logging around fetching slash commands and verifying the working directory. Consider removing or guarding these logs in production builds.
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 199df9f in 1 minute and 10 seconds. Click for details.
- Reviewed
37lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherInput.tsx:12
- Draft comment:
Explicitly importing React along with the hooks improves clarity and consistency. If you’re using the new JSX transform where an explicit React import isn’t required, consider if this extra import is necessary—but it’s a fine safeguard. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. humanlayer-wui/src/components/internal/SessionDetail/components/ResponseEditor.tsx:484
- Draft comment:
The new localWorkingDirRef provides a backwards-compatible fallback when no workingDirRef is passed. Verify that this design is intentional—when a workingDirRef is provided, its value will not be updated from the workingDir prop. Otherwise, the useEffect updating localWorkingDirRef (lines 491–494) correctly keeps the local ref in sync with workingDir. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_245B1CKg6Ax41kZp
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Contributor
Author
|
ok this is ready for review but make sure to spend some time driving it - slash commands for draft launcher were borked and so were (are still, technically -- but will be fixed shortly) @-mentioning files from the draft launcher |
Clean up console.log statements used for debugging working directory refs and draft creation flow. Convert console.error to logger.error for proper error handling.
… draft launcher - Add optional workingDir prop to FuzzyFileMentionList interface - Use passed workingDir as primary source, fall back to store value - Update ResponseEditor to pass effectiveWorkingDirRef.current to component - Apply same pattern as SlashCommandList fix for consistency - Fix linting issue in DraftLauncherInput catch block
Contributor
Author
🐛 Bug Fix: @-mention file autocomplete in Draft LauncherFixed an issue where the file autocomplete (@-mentions) was not working in the Draft Launcher. ProblemThe FuzzyFileMentionList component was only reading the working directory from the global store ( SolutionApplied the same pattern that was previously used to fix slash commands:
Testing
Commit: c8f929b |
The ModelSelector component was attempting to call updateSession with a null session ID when the user tried to change the model before creating a draft (by typing in title/prompt). This fix: - Checks if session.id exists before calling daemonClient.updateSession() - Still calls the onModelChange callback to store settings locally - Updates success toast messages to indicate settings are saved for new draft - Skips session refresh when no session exists yet The draft launcher will apply these stored model settings when the draft is eventually created through its existing sync mechanism. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
Fixed: Model selection error in draft launcherJust pushed a fix for an issue where selecting a model in the draft launcher before typing anything would throw an error. The ProblemWhen opening the draft launcher and trying to change the model through the 'select model' modal before typing anything into the title or prompt field, users would get: This happened because:
The SolutionModified
The draft launcher's existing sync mechanism will apply these stored model settings when the draft is eventually created. Commit: 961396c |
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to c8f929b in 3 minutes and 12 seconds. Click for details.
- Reviewed
2873lines of code in31files - Skipped
0files when reviewing. - Skipped posting
13draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/SessionDetailRouter.tsx:25
- Draft comment:
Good use of useEffect to redirect draft sessions. Consider adding a comment that the redirect is instantaneous to avoid accidental rendering of stale UI state. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The code already handles the redirect case well by returning null during redirect. The existing comments clearly explain the routing logic. Adding a comment about redirect timing seems unnecessary and could become outdated if React's behavior changes. The { replace: true } flag in navigate already implies this is a clean redirect. The comment might be trying to prevent future developers from adding code that could cause flicker during the redirect. The code structure with return null already prevents this issue, and the existing comments sufficiently document the behavior. Additional comments about React internals would be over-documentation. The comment should be deleted as it suggests adding unnecessary documentation when the code structure and existing comments already handle and explain the behavior well.
2. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:200
- Draft comment:
The lazy draft creation logic looks good. Ensure that handling of editor updates (triggering handleCreateDraft) does not fire repeatedly under rapid typing. It might be good to debounce if issues arise. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. humanlayer-wui/src/components/internal/SessionDetail/components/DraftLauncherForm.tsx:316
- Draft comment:
Nice use of refs to maintain mutable state (e.g. sessionIdRef). Consider a brief inline comment clarifying why session ref and localStorage are synchronized. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment appears to be confused - there isn't actually any direct synchronization between sessionIdRef and localStorage. The sessionIdRef is synchronized with the sessionId state variable, not localStorage. The comment seems to misunderstand what's happening in the code. Additionally, the code already has clear section headers and comments explaining the purpose of different sections. Could I be missing some indirect relationship between sessionIdRef and localStorage that would make this documentation valuable? Is there some non-obvious interaction I'm not seeing? No - after careful review, the comment is definitely incorrect. The sessionIdRef is only synchronized with the sessionId state variable, and this is already clearly documented with the "KEEP REFS IN SYNC WITH STATE" section header. The comment should be deleted because it is based on an incorrect understanding of the code. The synchronization that exists is already well-documented.
4. humanlayer-wui/src/components/internal/SessionDetail/components/ResponseEditor.tsx:484
- Draft comment:
The approach to pass and update the working directory via a local ref (effectiveWorkingDirRef) is a clean solution. Just ensure that changes to workingDir from parent are synchronized if needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the author to ensure synchronization, which is similar to asking them to double-check or verify something. It doesn't provide a specific suggestion or point out a specific issue. It violates the rule against asking the author to ensure behavior is intended.
5. humanlayer-wui/src/lib/daemon/http-client.ts:145
- Draft comment:
Good refactoring to require a workingDir parameter for slash commands instead of sessionId. Validate that the backend strictly enforces correct workingDir usage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to validate something, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue in the code.
6. humanlayer-wui/src/components/internal/SessionDetail/components/ModelSelector.tsx:49
- Draft comment:
The logic in getProviderAndModel is clear. For clarity, consider extracting provider‐specific logic into a helper function if it grows further. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. humanlayer-wui/src/hooks/useSessionLauncher.ts:63
- Draft comment:
The localStorage helper functions for saving model keys and provider look clean. Ensure that proper error handling is in place if localStorage access fails (e.g. in private mode). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. humanlayer-wui/src/pages/SessionTablePage.tsx:41
- Draft comment:
Refreshing sessions when viewMode is drafts is a good idea. Double-check that the dependency array includes all needed setter functions for consistency (though useStore selectors are stable). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to double-check the dependency array, which violates the rule against asking for confirmation or double-checking. It doesn't provide a specific suggestion or point out a clear issue.
9. humanlayer-wui/src/router.tsx:22
- Draft comment:
The route structure is now clear. The addition of /sessions/draft route helps separate draft and active sessions. Nice work. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only praises the clarity of the route structure without providing any actionable feedback or suggestions. It doesn't align with the rules for useful comments.
10. package.json:15
- Draft comment:
Updating @biomejs/biome to ^2.2.6 is appropriate. Ensure that all team members update dependencies accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about updating dependencies, which is not allowed according to the rules. It instructs team members to update to a specific version, which is a dependency-related comment.
11. test-slash-commands.md:1
- Draft comment:
The manual test plan is thorough and well-structured. It’s good to see explicit test cases for draft creation, file mentions, and navigation between drafts. It might be useful to add a note for testers regarding API key placeholders if custom models are configured. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking for a note to be added for testers, which falls under the category of asking the PR author to ensure something is tested or to provide additional information. This violates the rule against asking for confirmation or ensuring testing.
12. hld/sdk/typescript/src/generated/apis/SessionsApi.ts:702
- Draft comment:
Typographical issue: In the error message, the function is referenced as getSlashCommands(), but the method in this context is getSlashCommandsRaw(). Consider updating the error message for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
13. hld/session/manager.go:844
- Draft comment:
Minor Typo: In the TODO comment, consider using 'JSON-RPC' instead of 'JsonRPC' and 'SQLite' instead of 'sqlite' to match standard terminology. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_bmlQ2aZ49j8vufb5
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| await refreshSessions() | ||
| } else { | ||
| // Draft not found, navigate to new draft | ||
| console.error('Draft not found with ID:', draftId) |
Contributor
There was a problem hiding this comment.
The test page uses console logging and clears activeSessionDetail on unmount. Consider adding user feedback in case the draft is not found, rather than only logging to console.
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 961396c in 3 minutes and 32 seconds. Click for details.
- Reviewed
49lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. humanlayer-wui/src/components/internal/SessionDetail/components/ModelSelector.tsx:227
- Draft comment:
Guard updateSession call with session.id to avoid updating undefined sessions. Ensure skipping updates for non-persistent (draft) sessions is intentional. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. humanlayer-wui/src/components/internal/SessionDetail/components/ModelSelector.tsx:254
- Draft comment:
Toast message logic now differentiates new draft sessions. Confirm that 'Model selection saved for new draft' is the desired message when session.id is absent. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. humanlayer-wui/src/components/internal/SessionDetail/components/ModelSelector.tsx:267
- Draft comment:
Conditionally refreshing session details only if session.id exists is a good defensive check. Verify that draft sessions are properly handled elsewhere. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify something, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue in the code. Therefore, it should be removed.
Workflow ID: wflow_ZCADIHW5q6Fo5wsp
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Updated test file to match the API change introduced in commit 2604113 where slash commands now use working_dir parameter directly instead of looking up sessions by session_id. This fixes the failing tests in the hld package. - Changed all test requests to use WorkingDir field instead of SessionId - Removed unnecessary GetSession mocking as it's no longer needed - All tests now pass successfully
dexhorthy
approved these changes
Oct 16, 2025
…separate-routes-with-tanstack
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What problem(s) was I solving?
Draft sessions were tightly coupled with active sessions in the SessionDetailRouter, making it difficult to manage their distinct behaviors and lifecycle. Users experienced issues with:
What user-facing changes did I ship?
/sessions/draft?id=XXXinstead of/sessions/:id, providing clearer URL semanticsHow I implemented it
1. Created separate routing infrastructure
/sessions/draftroute inrouter.tsxwith optional?id=parameterDraftSessionPage.tsxas a dedicated wrapper component for draft sessionsSessionDetailRouter.tsxto redirect draft sessions to the new routeDraftLauncher.tsxtoDraftLauncherForm.tsxfor clarity2. Fixed lazy draft creation
DraftLauncherFormto track "dirty" state and only create drafts when content changes3. Enhanced navigation flow
useSessionLauncherto navigate to/sessions/draftwithout creating a sessionSessionTablePageto route draft sessions to the dedicated draft route4. Fixed model and proxy persistence
ModelSelectorandDraftLauncherFormcomponents5. Backend improvements
session.Infostruct to include EditorState and proxy configuration fieldsGetSessionInfoandListSessionsAPI handlers to return all session fieldsSlashCommandSourceconstant format6. Development tooling
react-hooks/exhaustive-depsrule to reduce false positive warningsHow to verify it
Manual Testing
/sessions/draftwithout creating a session/sessions/draft?id=XXXAutomated Testing
make build✓bun test- 186 tests passing, 0 failures ✓make check- All checks passing ✓Description for the changelog
Fixed draft session handling with dedicated routing, lazy draft creation, and improved model persistence. Draft sessions now use
/sessions/draftroute and are only created when users start typing. Model and proxy settings properly persist across sessions.Important
Separated draft and active session routes with improved draft handling and model persistence.
/sessions/draftroute inrouter.tsxfor draft sessions.DraftSessionPage.tsxfor handling draft sessions.SessionDetailRouter.tsxto redirect drafts to the new route.DraftLauncherForm.tsx.useSessionLauncherto navigate to/sessions/draft.ModelSelectorandDraftLauncherForm.session.Infostruct to include proxy configuration fields.react-hooks/exhaustive-depsrule.@biomejs/biomedependency inpackage.json.This description was created by
for 223c70b. You can customize this summary. It will automatically update as commits are pushed.
Additional fixes
Slash commands in draft sessions