-
Notifications
You must be signed in to change notification settings - Fork 15
Release 0.7.3: CLI recursion tracking #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Track depth server-side for sub_aleph via max_depth_seen in session - Pass current_depth through CLI backend responses - Skip phantom models/tokens/cost in output for CLI backends - Clean up README (remove stale RLM Updates section) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @Hmbown, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the behavior of Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces recursion tracking for CLI backends, which is a significant feature enhancement. The changes include tracking recursion depth, updating backend priority to prefer CLI, and improving output for CLI backends by omitting cost/token information. The documentation and tests have been updated accordingly, which is great to see.
I've found a critical issue related to session sharing for unsupported CLI backends that could lead to an UnboundLocalError, and a minor inconsistency in error reporting. Please see my detailed comments.
| share_session = _get_env_bool("ALEPH_SUB_QUERY_SHARE_SESSION", False) | ||
| if share_session and resolved_backend in {"claude", "codex", "gemini"}: | ||
| host = os.environ.get("ALEPH_SUB_QUERY_HTTP_HOST", "127.0.0.1") | ||
| port = _get_env_int("ALEPH_SUB_QUERY_HTTP_PORT", 8765) | ||
| path = os.environ.get("ALEPH_SUB_QUERY_HTTP_PATH", "/mcp") | ||
| server_name = os.environ.get( | ||
| "ALEPH_SUB_QUERY_MCP_SERVER_NAME", | ||
| "aleph_shared", | ||
| ).strip() or "aleph_shared" | ||
| ok, url_or_err = await self._ensure_streamable_http_server(host, port, path) | ||
| if not ok: | ||
| response = AlephResponse( | ||
| answer="", | ||
| success=False, | ||
| total_iterations=0, | ||
| max_depth_reached=0, | ||
| total_tokens=0, | ||
| total_cost_usd=0.0, | ||
| wall_time_seconds=time.perf_counter() - start_time, | ||
| trajectory=[], | ||
| error=f"Failed to start streamable HTTP server: {url_or_err}", | ||
| error_type="cli_error", | ||
| ) | ||
| else: | ||
| mcp_server_url = url_or_err | ||
| prompt = ( | ||
| f"{prompt}\n\n" | ||
| f"[MCP tools are available via the live Aleph server. " | ||
| f"Use context_id={context_id!r} when calling tools. " | ||
| f"Tools are prefixed with `mcp__{server_name}__`.]" | ||
| ) | ||
|
|
||
| if mcp_server_url is not None or not share_session: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential UnboundLocalError here due to flawed logic in handling session sharing for CLI backends.
If share_session is True but resolved_backend is not one of {"claude", "codex", "gemini"}, the if block for setting up mcp_server_url (lines 1139-1168) is skipped, and mcp_server_url remains None.
Consequently, the condition if mcp_server_url is not None or not share_session: at line 1170 evaluates to False. This causes the entire block that calls run_cli_sub_query and sets the response variable to be skipped. The function then proceeds without response being defined, which will lead to an UnboundLocalError when response is accessed later (e.g., line 1271).
To fix this, the logic should be adjusted to ensure run_cli_sub_query is called even if session sharing is requested for an unsupported backend (it should just run without sharing). A simple fix would be to remove the conditional check at line 1170 and un-indent the subsequent block, so the query is always executed for CLI backends unless an earlier error occurred.
| total_iterations=0, | ||
| max_depth_reached=0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When _ensure_streamable_http_server fails, the AlephResponse is created with total_iterations=0 and max_depth_reached=0. This is inconsistent with other error responses for CLI backends, which correctly use current_depth. To maintain consistency, these should be set to current_depth.
| total_iterations=0, | |
| max_depth_reached=0, | |
| total_iterations=current_depth, | |
| max_depth_reached=current_depth, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| query=query, | ||
| context_slice=context_slice, | ||
| context_id=context_id, | ||
| current_depth=session.max_depth_seen + 1 if session else 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depth tracking increments incorrectly for sibling calls
Medium Severity
The current_depth calculation uses session.max_depth_seen + 1, which means sequential sub_aleph calls at the same logical level get different depths. For example, two sibling calls from the root would report depths 2 and 3 respectively, instead of both being at depth 2. The max_depth_seen is intended to track the maximum depth reached, not to calculate the current call's depth level.
Additional Locations (1)
| if len(raw) >= 2 and ((raw[0] == raw[-1] == '"') or (raw[0] == raw[-1] == "'")): | ||
| raw = raw[1:-1].strip() | ||
| return raw, True | ||
| return text.strip(), False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated regex patterns and extraction logic across modules
Low Severity
The regex patterns _FINAL_RE and _FINAL_VAR_RE are duplicated identically between aleph/core.py and aleph/mcp/local_server.py. The _extract_final_answer function in local_server.py also reimplements the same extraction logic found in _extract_final and _extract_final_var methods in core.py. These could be consolidated into a shared utility module.
Additional Locations (1)
| tasks: list[dict[str, Any]] = field(default_factory=list) | ||
| task_counter: int = 0 | ||
| # Recursion depth tracking for sub_aleph | ||
| max_depth_seen: int = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Session save/load omits new max_depth_seen field
Medium Severity
The new max_depth_seen field added to _Session is not included in _session_to_payload when saving sessions, and is not restored when loading sessions. This causes the depth tracking state to be lost when sessions are persisted via save_session/load_session, resetting to the default value of 1 after reload.
Note
codex→gemini→claude) over API; updated detection logic, docs (README.md,docs/prompts/aleph.md), and tests.sub_alephpath: Adds a single-shot CLI execution flow with prompt construction,FINAL(...)/FINAL_VAR(...)parsing, optional MCP session sharing, and context truncation safeguards; includes backend/truncation in metadata and evidence.models/tokens/costinsub_alephresults and surfacebackendand wall time; show truncated-context note when applicable.session.max_depth_seen; includecurrent_depthinsub_alephcalls.query; minor example fixes in system prompt docs.--skip-git-repo-checkto Codex CLI invocations.0.7.3.Written by Cursor Bugbot for commit 5245c89. This will update automatically on new commits. Configure here.