Improve forum topic session routing and add interactive /history pager#6
Merged
Merged
Conversation
WalkthroughThis pull request introduces interactive session history browsing ( Changes
Sequence Diagram(s)sequenceDiagram
participant User as Telegram User
participant Bot as Telecodex Bot
participant Store as Session Store
participant Codex as Codex API
User->>Bot: /history (in work topic)
Bot->>Store: ensure_resolved_session(user)
Store-->>Bot: SessionRecord with codex_thread_id
Bot->>Codex: fetch history from thread
Codex-->>Bot: CodexHistoryEntry list
Bot->>Bot: render_history_page(entries, index=0)
Bot->>User: Display page 0 with prev/next buttons
User->>Bot: Click next button (callback: his:thread_id:1)
Bot->>Bot: parse_history_callback_data()
Bot->>Store: verify current codex_thread_id
Store-->>Bot: SessionRecord
Bot->>Bot: validate callback thread matches session
alt Thread mismatch detected
Bot->>Bot: render_stale_history_page()
Bot->>User: Display stale history warning
else Thread matches
Bot->>Codex: fetch history again
Codex-->>Bot: CodexHistoryEntry list
Bot->>Bot: render_history_page(entries, index=1)
Bot->>User: Display page 1 with pagination
end
sequenceDiagram
participant User as Telegram User
participant Bot as Telecodex Bot
participant Store as Session Store
User->>Bot: /status (in work topic)
Bot->>Store: ensure_resolved_session(user)
Store-->>Bot: SessionRecord
Bot->>Bot: format_session_status(session)
Bot->>User: Display Telegram session, Codex thread, model, settings
User->>Bot: /status (in dashboard root)
Bot->>Bot: Detect dashboard_root context
Bot->>User: Return instructive message: use /status in work topic
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
src/app.rs (1)
1428-1455: Avoid reloading the full thread on every pager click.
render_history_page()rereads the entire history withusize::MAXand rebuilds the assistant page list for each callback. On long Codex threads, Prev/Next will degrade into repeated full disk scans and slow message edits. A small cache keyed by(codex_thread_id, message_id)or a bounded precomputed page list would keep navigation responsive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app.rs` around lines 1428 - 1455, render_history_page currently calls read_thread_history(..., usize::MAX) and assistant_history_pages on every pager callback which causes repeated full-disk scans; change it to look up a short-lived/bounded in-memory cache (keyed by codex_thread_id and message_id or codex_thread_id and initial render timestamp) and only reload from read_thread_history when the cache miss occurs or the cached entry is stale/evicted; populate the cache when you first render the history page (in the same function or its caller) with the precomputed pages (the assistant_history_pages result) and have subsequent calls to render_history_page read pages from that cache and use history_keyboard/edit/send functions as before, ensuring the cache is bounded (LRU or size limit) and entries expire to avoid unbounded memory growth.
🤖 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/app/presentation.rs`:
- Around line 761-794: The code currently shows a "codex session title" even
when session.codex_thread_id is None because current_session_label() falls back
to the Telegram topic; update format_session_status so codex_title is derived
only when a Codex thread is actually bound: check session.codex_thread_id (e.g.,
via session.codex_thread_id.is_some() or matching the Option) and set
codex_title = escape_markdown_label(¤t_session_label(...)) only when
present, otherwise set an explicit marker like "unbound" or "fresh" (escaped),
and leave the rest of the formatting (codex_thread variable/fields) unchanged so
the status accurately reflects an unbound session.
---
Nitpick comments:
In `@src/app.rs`:
- Around line 1428-1455: render_history_page currently calls
read_thread_history(..., usize::MAX) and assistant_history_pages on every pager
callback which causes repeated full-disk scans; change it to look up a
short-lived/bounded in-memory cache (keyed by codex_thread_id and message_id or
codex_thread_id and initial render timestamp) and only reload from
read_thread_history when the cache miss occurs or the cached entry is
stale/evicted; populate the cache when you first render the history page (in the
same function or its caller) with the precomputed pages (the
assistant_history_pages result) and have subsequent calls to render_history_page
read pages from that cache and use history_keyboard/edit/send functions as
before, ensuring the cache is bounded (LRU or size limit) and entries expire to
avoid unbounded memory growth.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 133da092-44ae-48fc-81e1-2f656e8afea4
📒 Files selected for processing (13)
README.mdREADME.ru.mdsrc/app.rssrc/app/forum.rssrc/app/io.rssrc/app/presentation.rssrc/app/support.rssrc/app/tests.rssrc/app/turns.rssrc/codex_history.rssrc/commands.rssrc/store.rssrc/telegram.rs
💤 Files with no reviewable changes (1)
- src/app/support.rs
Comment on lines
+761
to
+794
| pub(super) fn format_session_status( | ||
| session: &crate::models::SessionRecord, | ||
| chat: &crate::telegram::Chat, | ||
| ) -> String { | ||
| let telegram_title = escape_markdown_label(&session_title_label(session, chat)); | ||
| let codex_title = escape_markdown_label(¤t_session_label(session, chat)); | ||
| let state = if session.busy { "busy" } else { "idle" }; | ||
| let codex_thread = session | ||
| .codex_thread_id | ||
| .as_deref() | ||
| .map(short_codex_thread_id) | ||
| .unwrap_or_else(|| "new".to_string()); | ||
| let model = session.model.as_deref().unwrap_or("default"); | ||
| let reasoning = session.reasoning_effort.as_deref().unwrap_or("default"); | ||
| let prompt = if session | ||
| .session_prompt | ||
| .as_deref() | ||
| .map(str::trim) | ||
| .filter(|value| !value.is_empty()) | ||
| .is_some() | ||
| { | ||
| "set" | ||
| } else { | ||
| "none" | ||
| }; | ||
|
|
||
| format!( | ||
| "**Current Telegram session:** {telegram_title}\n- codex session title: {codex_title}\n- state: `{state}`\n- cwd: `{}`\n- codex thread: `{}`\n- model: `{model}`\n- reasoning: `{reasoning}`\n- approval: `{}`\n- sandbox: `{}`\n- search: `{}`\n- prompt: `{prompt}`", | ||
| session.cwd.display(), | ||
| codex_thread, | ||
| session.approval_policy, | ||
| session.sandbox_mode, | ||
| session.search_mode.as_codex_value(), | ||
| ) |
There was a problem hiding this comment.
Don't label an unbound topic as a Codex session.
When session.codex_thread_id is None, current_session_label() falls back to the Telegram topic title, so /status can report a "codex session title" even immediately after /new or /clear. Show an explicit fresh/unbound state here, or omit that field until a Codex thread is selected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/presentation.rs` around lines 761 - 794, The code currently shows a
"codex session title" even when session.codex_thread_id is None because
current_session_label() falls back to the Telegram topic; update
format_session_status so codex_title is derived only when a Codex thread is
actually bound: check session.codex_thread_id (e.g., via
session.codex_thread_id.is_some() or matching the Option) and set codex_title =
escape_markdown_label(¤t_session_label(...)) only when present, otherwise
set an explicit marker like "unbound" or "fresh" (escaped), and leave the rest
of the formatting (codex_thread variable/fields) unchanged so the status
accurately reflects an unbound session.
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.
Что меняется
PR улучшает поведение Telecodex в Telegram forum mode и добавляет удобный просмотр истории выбранной Codex-сессии.
Основные изменения
/statusбольше не пробрасывается в Codex CLI и обрабатывается самим Telecodex/useмог тихо затираться/newи/clearбольше не перетирается фоновым sync/copyтеперь соответствует реально выбранной Codex-сессии/sessionsв root dashboard больше не создаёт ощущение самопроизвольного переключения/historyс интерактивным pager:Зачем
В forum dashboard были странности с routing/UX:
/useвыбранная сессия могла незаметно переключиться обратно/statusместами уходил в сам Codex вместо bot-layer/copyмог показывать ответ не из той сессии/sessionsв dashboard и work topic вёл себя слишком неоднозначноЭтот PR делает поведение topic-сессий предсказуемее и добавляет нормальный способ смотреть историю выбранного thread прямо из Telegram.
Проверка
Проверено локально:
cargo test-> все тесты проходят/sessions//use/status/copyиз выбранной сессии/historyс кнопкамиPrev/NextОграничения
/sessionsпо-прежнему контекстная команда:cwd/historyсейчас показывает только итоговые assistant-сообщения, а не полный transcript