fix(memory): address code review feedback for auto-memory recall#3866
Merged
wenshao merged 5 commits intoMay 7, 2026
Conversation
The model-driven relevance selector (selectRelevantAutoMemoryDocumentsByModel) currently uses the main session model for its LLM call. Since this is a background side-query that runs in parallel with the user's main request, route it to config.getFastModel() instead — consistent with sessionRecap, sessionTitle, toolUseSummary, and forkedAgent which all prefer the fast model for background work. When no fast model is configured, getFastModel() returns undefined and runSideQuery falls back to config.getModel(), so behavior is unchanged for users without a fast model set. Co-Authored-By: Claude <noreply@anthropic.com>
eee67f1 to
4387f95
Compare
wenshao
reviewed
May 7, 2026
| * @param p - The path to expand. | ||
| * @returns The expanded path. | ||
| */ | ||
| export function expandHomeDir(p: string): string { |
Collaborator
There was a problem hiding this comment.
[Suggestion] This adds expandHomeDir() as a new exported core helper, but the existing CLI implementation in packages/cli/src/ui/commands/directoryCommand.tsx is still the only production call site. That leaves the new export unused while the same path-expansion behavior now exists in two places, so future changes to ~ or %userprofile% handling can drift and the API intent is unclear.
Either remove this new helper if it is not needed for the memory-recall fix, or complete the extraction by importing the core helper from the CLI command and deleting the local duplicate implementation.
— gpt-5.5 via Qwen Code /review
wenshao
approved these changes
May 7, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
TaimoorSiddiquiOfficial
pushed a commit
to TaimoorSiddiquiOfficial/HopCode
that referenced
this pull request
May 8, 2026
…nLM#3866) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DragonnZhang
pushed a commit
that referenced
this pull request
May 8, 2026
* fix(memory): route auto-memory recall selector to fast model The model-driven relevance selector (selectRelevantAutoMemoryDocumentsByModel) currently uses the main session model for its LLM call. Since this is a background side-query that runs in parallel with the user's main request, route it to config.getFastModel() instead — consistent with sessionRecap, sessionTitle, toolUseSummary, and forkedAgent which all prefer the fast model for background work. When no fast model is configured, getFastModel() returns undefined and runSideQuery falls back to config.getModel(), so behavior is unchanged for users without a fast model set. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address code review feedback for auto-memory recall * fix(core): move expandHomeDir to core to resolve import errors in CLI * test(memory): fix document path in mock to match test data * test(core): add unit tests for expandHomeDir --------- Co-authored-by: Claude <noreply@anthropic.com>
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
…nLM#3866) * fix(memory): route auto-memory recall selector to fast model The model-driven relevance selector (selectRelevantAutoMemoryDocumentsByModel) currently uses the main session model for its LLM call. Since this is a background side-query that runs in parallel with the user's main request, route it to config.getFastModel() instead — consistent with sessionRecap, sessionTitle, toolUseSummary, and forkedAgent which all prefer the fast model for background work. When no fast model is configured, getFastModel() returns undefined and runSideQuery falls back to config.getModel(), so behavior is unchanged for users without a fast model set. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address code review feedback for auto-memory recall * fix(core): move expandHomeDir to core to resolve import errors in CLI * test(memory): fix document path in mock to match test data * test(core): add unit tests for expandHomeDir --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
Surgical fixes for auto-memory recall: