Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the sirchmunk local search from a pre-turn RAG injection into an on-demand tool, adding support for streaming tool logs and detailed UI outputs. Key feedback identifies a fragile heuristic for detecting log lines that could cause hangs, suggesting a more robust protocol by wrapping final results in dictionaries. Other points include concerns over the simplified system prompt potentially degrading performance and the addition of unused dead code in the CLI.
ms_agent/agent/llm_agent.py
Outdated
| if isinstance(item, dict) or not isinstance(item, str) or \ | ||
| not self._is_log_line(item): |
There was a problem hiding this comment.
This logic to differentiate between intermediate log lines and final tool results is fragile. It relies on _is_log_line, a heuristic that assumes any string not matching specific error patterns is a log. This can cause the agent to hang if a tool's final result is a simple string (e.g., "OK"), as it would be misinterpreted as a log line. The agent would then wait indefinitely for a final result. This is especially problematic for non-streaming tools that use the default ToolBase.call_tool_streaming implementation.
ms_agent/cli/run.py
Outdated
| def load_env_file(self): | ||
| """Load environment variables from .env file in current directory.""" | ||
| env_file = os.path.join(os.getcwd(), '.env') | ||
| if os.path.exists(env_file): | ||
| with open(env_file, 'r') as f: | ||
| for line in f: | ||
| line = line.strip() | ||
| if line and not line.startswith('#') and '=' in line: | ||
| key, value = line.split('=', 1) | ||
| key = key.strip() | ||
| value = value.strip() | ||
| # Only set if not already set in environment | ||
| if key not in os.environ: | ||
| os.environ[key] = value | ||
| logger.debug(f'Loaded {key} from .env file') | ||
|
|
There was a problem hiding this comment.
The method load_env_file is defined but appears to be unused. The _execute_with_config method calls Env.load_dotenv_into_environ to load environment variables. This new method seems to be dead code and should be removed to avoid confusion and code clutter.
1 participant
Change Summary
Related issue number
Checklist
pre-commit installandpre-commit run --all-filesbefore git commit, and passed lint check.