fix: final answer rendering and timing leaks#129
Merged
F16shen merged 6 commits intoAI-Shell-Team:mainfrom Apr 23, 2026
Merged
Conversation
Contributor
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds PTY startup handshake state exposure in PTYManager, makes PTYAIShell consume those signals (avoiding unconditional sleeps), restricts parent callback forwarding via an LLM event allowlist, and changes TTFT/streaming to record only on final content deltas. Changes
Sequence DiagramsequenceDiagram
participant Shell as PTYAIShell
participant PTYMgr as PTYManager
participant Proto as PTY Handshake Protocol
Shell->>PTYMgr: start()
activate PTYMgr
PTYMgr->>PTYMgr: reset startup flags
PTYMgr->>Proto: initialize PTY
loop handshake polling
Proto-->>PTYMgr: control events (session_ready, prompt_ready, cwd, ps2)
PTYMgr->>PTYMgr: decode events, record readiness, cwd, ps2, protocol issues
end
PTYMgr-->>Shell: start() returns with startup_* and startup_cwd set
deactivate PTYMgr
Shell->>Shell: _setup_pty()
Shell->>PTYMgr: query startup_ready, startup_session_ready, startup_cwd
alt startup signals present
Shell->>Shell: set backend cwd, _backend_session_ready=True, shell_phase="editing", skip sleep
else fallback
Shell->>Shell: perform legacy sleep delay
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
F16shen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/aish/shell/runtime/app.py`:
- Around line 1130-1136: The branch only sets _backend_session_ready when
_pty_manager.startup_ready is true, which misses the case where
PTYManager.start() consumed session_ready but didn't set startup_ready; change
the condition to set self._backend_session_ready = True if either
self._pty_manager.startup_ready or self._pty_manager.session_ready is truthy
(i.e., check both flags/properties), so replace the existing if-block that sets
_backend_session_ready and _shell_phase (or set _backend_session_ready
independently) to preserve session readiness even when startup_ready is false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4869219-4a1a-47dc-9cec-8924aba13564
📒 Files selected for processing (6)
src/aish/llm/agents.pysrc/aish/shell/runtime/app.pysrc/aish/terminal/pty/manager.pytests/shell/runtime/test_shell_pty_core.pytests/terminal/pty/test_pty_control_protocol.pytests/tools/test_final_answer.py
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.
Summary
思考: Xslines.SystemDiagnoseAgentevent forwarding to tool/error/cancel/interaction events.Change Type
Scope
User-visible Changes
思考: Xssummaries no longer appear when a turn only emitted non-final preview content.system_diagnose_agentno longer leaks nested lifecycle/content events into the outer shell.Compatibility
Testing
/home/lixin/workspace/aishell/aish/.venv/bin/python -m pytest tests/shell/runtime/test_shell_pty_core.py -k "content_streamed or ttft_timing_records_on_first_content_delta or ttft_timing_preserves_state_across_generations"/home/lixin/workspace/aishell/aish/.venv/bin/python -m pytest tests/tools/test_final_answer.py -k "filters_nested_content_and_lifecycle_events or mocked_llm_with_final_answer_after_bash or end_to_end_nested_agent_execution"/home/lixin/workspace/aishell/aish/.venv/bin/python -m pytest tests/shell/runtime/test_shell_pty_core.py -k "ttft_timing_records_on_first_content_delta or non_final_content_delta_does_not_record_ttft or op_end_does_not_render_ttft_for_non_final_preview_only or final_content_delta_marks_content_streamed"Checklist
Summary by CodeRabbit
Performance Improvements
Reliability Improvements
Robustness
Tests