fix(cli): honour SURREAL_MEMORY_BRAIN when --brain is omitted - #90
Open
RobertSigmundsson wants to merge 1 commit into
Open
fix(cli): honour SURREAL_MEMORY_BRAIN when --brain is omitted#90RobertSigmundsson wants to merge 1 commit into
RobertSigmundsson wants to merge 1 commit into
Conversation
CLI commands resolved the brain as `brain or config.current_brain` before calling get_storage(), always passing a non-None name. That made get_storage() take its explicit-arg branch and skip its own env-var check, so SURREAL_MEMORY_BRAIN was silently ignored and commands ran against the wrong brain (falling back to 'default'). Add a central resolve_brain(brain, config, *, default=None) helper (explicit-arg > env var > default/config) and route get_storage() plus all ten CLI call sites through it: consolidate, decay, lifecycle (tools.py); export, import (shortcuts.py); brain list/export/import/health (brain.py). Also drops the duplicated SURREAL_MEMORY_BRAIN lookup in get_storage(). Add tests/unit/test_cli_resolve_brain.py covering the priority order and the empty-env-var edge case. (cherry picked from commit 1bdc7b2)
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.
Problem
get_storagehas its own env-var fallback for the brain name, but it only fires when the caller passesNone. Several CLI commands need the brain name before callingget_storage(to echo progress), andthey resolve it with the
brain or config.current_brainpattern. That expression is neverNone, soget_storageskips its own env check andSURREAL_MEMORY_BRAINis silently ignored — the commandprints and operates on the config-file brain instead of the requested one.
Change
Add a single
resolve_brain(brain, config, *, default=None)helper incli/_helpers.pyimplementing thedocumented priority — explicit arg > env var > default/config — and route
brain.py,shortcuts.pyand
tools.pythrough it. The helper's docstring states why thebrain or config.current_brainpatternmust not be used directly.
Why upstream benefits
SURREAL_MEMORY_BRAINis a documented knob; today it works for some commands and not others, with noerror to indicate which. One helper makes the precedence rule single-sourced and testable.
Side effect worth noting
maincurrently has a redundant expression at three call sites, e.g.cli/_helpers.py:90:— the same lookup on both sides of the
or(alsocli/commands/brain.py:39and:87). Harmless, but aclear leftover. Routing through
resolve_brainremoves all three.Evidence
origin/mainimport surreal_memoryruff checkruff format --checkmypy src/ --ignore-missing-importspytest -m "not stress" -n autoNew file:
tests/unit/test_cli_resolve_brain.py(+59) covering all three precedence branches.Risks / notes for the reviewer
SURREAL_MEMORY_BRAINand relied on those specificcommands ignoring it will now see them honour it. That was the bug.