fix(platform/copilot): revert forward pagination, add visibility guarantee for blank chat - #12831
Merged
Merged
Conversation
…antee for blank chat PR #12796 changed completed sessions to load from sequence 0 forward, which broke the standard chat UX (users land at the beginning instead of the end). This reverts the forward pagination approach and instead adds a visibility guarantee: after fetching the newest messages, if the entire page is tool messages (invisible in UI), expand backward until at least one user/assistant message is included so the chat never appears blank. Backend: - Remove after_sequence, from_start, forward_paginated, newest_sequence - Always use backward (newest-first) pagination for all sessions - Add _expand_for_visibility helper that scans up to 200 messages back - Extract _expand_tool_boundary helper from inline code Frontend: - Remove forwardPaginated/newestSequence plumbing from all hooks - Remove bottom LoadMoreSentinel and forward pagination props - Simplify useLoadMoreMessages to backward-only - Always prepend paged messages before current messages
majdyz
requested review from
Bentlybro and
ntindle
and removed request for
a team
April 17, 2026 11:27
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
github-actions
Bot
added
platform/frontend
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
WalkthroughThe PR removes forward-pagination (after_sequence/newest_sequence/forward_paginated) across backend and frontend, consolidating pagination to backward-only cursoring via limit and before_sequence; backend pagination adds tool-boundary and visibility-expansion helpers; frontend simplifies hooks/components to always prepend older pages and show "Load older messages." Changes
Sequence Diagram(s)sequenceDiagram
participant FE as Frontend (Chat page)
participant API as Backend API (GET /sessions/{id})
participant DB as DB Layer (get_chat_messages_paginated)
FE->>API: GET /sessions/{id}?limit=N (initial load, no before_sequence)
API->>DB: get_chat_messages_paginated(limit=N, before_sequence=None)
DB-->>API: messages (newest-first), oldest_sequence, has_more_messages
API-->>FE: SessionDetailResponse (messages, oldest_sequence, has_more_messages, active_stream)
FE->>API: GET /sessions/{id}?before_sequence=OLD_SEQ (load more)
API->>DB: get_chat_messages_paginated(limit=N, before_sequence=OLD_SEQ)
DB-->>API: messages (newest-first, expanded for tool-boundary/visibility), oldest_sequence, has_more_messages
API-->>FE: Paginated page (messages, oldest_sequence, has_more_messages)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/db.py`:
- Around line 226-231: The visibility-expansion branch leaves has_more stale
when prepend exists but reaches sequence 0; update the logic in the block that
manipulates prepend/results/has_more (the code using variables prepend, results,
and has_more) so that after prepending you set has_more = (prepend[0].sequence >
0) instead of only setting it True; i.e., ensure has_more is explicitly set
based on prepend[0].sequence so it becomes False when the expansion hit the
conversation start (sequence == 0).
- Around line 176-180: The code currently only ever flips has_more to True when
prepending boundary_msgs, which can leave a stale True if the expansion reached
sequence 0; after you merge boundary_msgs into results (the block handling
boundary_msgs + results), recompute has_more from the new oldest message by
setting has_more = (boundary_msgs[0].sequence > 0) so the returned has_more
reflects the true availability of older messages (references: variables
boundary_msgs, results, has_more and the expansion logic in
_expand_tool_boundary).
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useLoadMoreMessages.ts:
- Around line 128-143: The current logic may set oldestSequence from
response.data.oldest_sequence even when newer merged messages were sliced,
leaving oldestSequence pointing at a discarded message; after building the
merged array inside setPagedRawMessages (using newRaw, prev and
MAX_OLDER_MESSAGES) derive the actual oldest sequence from the final merged
array (e.g., merged[0]?.sequence or equivalent field) and pass that to
setOldestSequence (falling back to response.data.oldest_sequence or null if
missing), and add a brief inline comment explaining this protects against stale
cursors if truncation occurs; reference setPagedRawMessages, newRaw,
pagedRawMessages, MAX_OLDER_MESSAGES and setOldestSequence.
🪄 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: f4c0b039-a1a8-4141-a5fb-575fe1e6e6ee
📒 Files selected for processing (16)
autogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/api/features/chat/routes_test.pyautogpt_platform/backend/backend/copilot/db.pyautogpt_platform/backend/backend/copilot/db_test.pyautogpt_platform/frontend/src/app/(platform)/copilot/CopilotPage.tsxautogpt_platform/frontend/src/app/(platform)/copilot/__tests__/useChatSession.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/__tests__/useCopilotPage.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/__tests__/useLoadMoreMessages.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatContainer/ChatContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/__tests__/ChatMessagesContainer.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/__tests__/LoadMoreSentinel.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.tsautogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.tsautogpt_platform/frontend/src/app/api/openapi.json
💤 Files with no reviewable changes (4)
- autogpt_platform/frontend/src/app/(platform)/copilot/CopilotPage.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/tests/LoadMoreSentinel.test.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatContainer/ChatContainer.tsx
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #12831 +/- ##
==========================================
- Coverage 65.38% 65.37% -0.01%
==========================================
Files 1848 1848
Lines 137517 137391 -126
Branches 14758 14739 -19
==========================================
- Hits 89918 89825 -93
+ Misses 44805 44771 -34
- Partials 2794 2795 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
autogpt_platform/backend/backend/copilot/db_test.py (2)
229-233: Assertion is trivially true.
[m.sequence for m in page.messages][0] <= 3holds for any page whose oldest message has sequence ≤ 3, which is already guaranteed by the mock input (sequences 3/4/5). It doesn't actually verify that visibility expansion was skipped. Consider assertingfind_many.call_count == 0(the page already contains visibleuser/assistantmessages and the first message is not a tool, so neither boundary nor visibility expansion should fire), matching the pattern intest_no_boundary_expansion_when_first_msg_not_toolat Line 442.♻️ Suggested tightening
- # Boundary expansion might fire (oldest is tool), but NOT visibility - assert [m.sequence for m in page.messages][0] <= 3 + # First (oldest) msg is 'user', so neither boundary nor visibility expansion fires + assert find_many.call_count == 0 + assert [m.sequence for m in page.messages] == [3, 4, 5]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/db_test.py` around lines 229 - 233, The assertion checking "[m.sequence for m in page.messages][0] <= 3" is trivial given the mock; instead verify that visibility expansion did not run by asserting the mocked DB fetch was not called again. Replace that trivial assertion with an assertion on the mock used for message fetching (e.g., assert find_many.call_count == 0) after calling get_chat_messages_paginated(SESSION_ID, limit=3), and optionally assert the first message in page.messages has role 'user' or 'assistant' to match the test pattern from test_no_boundary_expansion_when_first_msg_not_tool.
208-211: Weak assertion in visibility-expansion test.
"assistant" in roles or "user" in rolesonly confirms a visible role was appended somewhere; it doesn't verify the expanded messages (seq 6–9) were actually prepended or that the original tool messages (10–12) remain. Consider asserting the full expected sequence list and thatpage.messages[0].role == "assistant"andpage.messages[0].sequence == 6to lock in the visibility-expansion contract.♻️ Suggested tightening
- # Should include the expanded messages + original tool messages - roles = [m.role for m in page.messages] - assert "assistant" in roles or "user" in roles - assert page.has_more is True + # Should include the expanded messages + original tool messages + assert [m.sequence for m in page.messages] == [6, 7, 8, 9, 10, 11, 12] + assert page.messages[0].role == "assistant" + assert page.has_more is True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/db_test.py` around lines 208 - 211, The test currently uses a weak check ("assistant" in roles or "user" in roles); update it to assert the exact expected visibility-expanded ordering by checking page.messages contains the prepended expanded messages (sequences 6–9) followed by the original tool messages (sequences 10–12), e.g. assert page.messages[0].role == "assistant" and page.messages[0].sequence == 6, assert the next messages have sequences 7–9 in order, and assert that sequences 10–12 still exist later in page.messages; keep the existing page.has_more is True assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/db_test.py`:
- Around line 229-233: The assertion checking "[m.sequence for m in
page.messages][0] <= 3" is trivial given the mock; instead verify that
visibility expansion did not run by asserting the mocked DB fetch was not called
again. Replace that trivial assertion with an assertion on the mock used for
message fetching (e.g., assert find_many.call_count == 0) after calling
get_chat_messages_paginated(SESSION_ID, limit=3), and optionally assert the
first message in page.messages has role 'user' or 'assistant' to match the test
pattern from test_no_boundary_expansion_when_first_msg_not_tool.
- Around line 208-211: The test currently uses a weak check ("assistant" in
roles or "user" in roles); update it to assert the exact expected
visibility-expanded ordering by checking page.messages contains the prepended
expanded messages (sequences 6–9) followed by the original tool messages
(sequences 10–12), e.g. assert page.messages[0].role == "assistant" and
page.messages[0].sequence == 6, assert the next messages have sequences 7–9 in
order, and assert that sequences 10–12 still exist later in page.messages; keep
the existing page.has_more is True assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cfb16e7-4466-498c-9f31-08546522a0f3
📒 Files selected for processing (1)
autogpt_platform/backend/backend/copilot/db_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: check API types
- GitHub Check: integration_test
- GitHub Check: end-to-end tests
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.13)
- GitHub Check: Seer Code Review
- GitHub Check: Analyze (python)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/db_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/db_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/db_test.py
🧠 Learnings (16)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12796
File: autogpt_platform/backend/backend/api/features/chat/routes.py:504-527
Timestamp: 2026-04-16T12:33:42.787Z
Learning: In `autogpt_platform/backend/backend/api/features/chat/routes.py`, `get_session` (PR `#12796`, commit 3771bfad9c1) closes the TOCTOU race between the initial `stream_registry.get_active_session()` pre-check and `get_chat_messages_paginated()` with a post-check re-verification: after the DB fetch, if `is_initial_load and active_session is not None`, it calls `get_active_session` a second time; if `post_active is None` (stream completed during the window), it resets `from_start=True`, `forward_paginated=True`, and re-fetches messages from sequence 0. Do NOT flag the double `get_active_session` call pattern as redundant — it is the intentional TOCTOU mitigation for pagination direction selection.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/copilot/pending_messages.py:52-64
Timestamp: 2026-04-14T14:36:22.396Z
Learning: In `autogpt_platform/backend/backend/copilot` (PR `#12773`, commit d7bced0c6): when draining pending messages into `session.messages`, each message's text is sanitized via `strip_user_context_tags` before persistence to prevent user-controlled `<user_context>` injection from bypassing the trusted server-side context prefix. Additionally, if `upsert_chat_session` fails after draining, the drained `PendingMessage` objects are requeued back to Redis to avoid silent message loss. Do NOT flag the drain-then-requeue pattern as redundant — it is the intentional failure-resilience strategy for the pending buffer.
Learnt from: kcze
Repo: Significant-Gravitas/AutoGPT PR: 12328
File: autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts:49-61
Timestamp: 2026-03-11T08:40:59.673Z
Learning: In `autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts`, clearing `olderMessages` (and resetting `oldestSequence`/`hasMore`) when `initialOldestSequence` shifts on the same session is intentional. Pages already fetched were based on a now-stale cursor; retaining them risks sequence gaps or duplicates. `ScrollPreserver` keeps the currently visible viewport intact, so only unvisited older pages are dropped. This is a deliberate safe-refetch design tradeoff.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12797
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1991-2021
Timestamp: 2026-04-15T13:44:31.808Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py` (`_run_stream_attempt`), the pre-create block (PR `#12797`) intentionally does NOT call `state.transcript_builder.append_assistant(...)` when inserting the empty assistant placeholder into `ctx.session.messages`. The transcript is left ending at the `tool_result` entry (N entries) while `message_count` metadata is N+1. This mismatch is benign and deliberate: on the next `--resume`, the SDK sees the transcript ending at `tool_result` and correctly regenerates the assistant response. Pre-appending the assistant turn to the transcript would suppress regeneration while leaving `session.messages[-1].content = ""` permanently (worse outcome). On the gap-fallback path, `transcript_msg_count (N+1) >= msg_count-1 (N)` means no gap is injected for the empty placeholder, which is correct because injecting an empty assistant message as context would mislead the SDK. Do NOT flag this transcript/message_count discrepancy as a bug.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12740
File: autogpt_platform/frontend/src/app/api/openapi.json:0-0
Timestamp: 2026-04-13T14:19:19.341Z
Learning: Repo: Significant-Gravitas/AutoGPT — autogpt_platform
When adding new CoPilot tool response models (e.g., ScheduleListResponse, ScheduleDeletedResponse), update backend/api/features/chat/routes.py to include them in the ToolResponseUnion so the frontend’s autogenerated openapi.json dummy export (/api/chat/schema/tool-responses) exposes them for codegen. Do not hand-edit frontend/src/app/api/openapi.json.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12445
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1071-1072
Timestamp: 2026-03-17T06:48:26.471Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), the AI SDK enforces `z.strictObject({type, errorText})` on SSE `StreamError` responses, so additional fields like `retryable: bool` cannot be added to `StreamError` or serialized via `to_sse()`. Instead, retry signaling for transient Anthropic API errors is done via the `COPILOT_RETRYABLE_ERROR_PREFIX` constant prepended to persisted session messages (in `ChatMessage.content`). The frontend detects retryable errors by checking `markerType === "retryable_error"` from `parseSpecialMarkers()` — no SSE schema changes and no string matching on error text. This pattern was established in PR `#12445`, commit 64d82797b.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12814
File: autogpt_platform/backend/backend/copilot/model.py:661-679
Timestamp: 2026-04-16T13:28:17.261Z
Learning: In `autogpt_platform/backend/backend/copilot/model.py` (PR `#12814`, commit 259d37083): `append_and_save_message` acquires `_get_session_lock` — a redis-py built-in Lock at key `copilot:session_lock:{session_id}` (timeout=10s, blocking_timeout=2s) — to serialize concurrent writers across replicas. On Redis failure the lock is skipped with a warning and the function continues. Inside the lock it re-fetches the session via `get_chat_session` (cache-first), performs an idempotency check (`session.messages[-1].role == message.role and session.messages[-1].content == message.content`), and returns early if matched. On successful DB write but failed cache write, it calls `invalidate_session_cache(session_id)` (the pre-existing best-effort helper) to evict the stale cache entry so subsequent retries fall back to the authoritative DB. Do NOT expect `asyncio.Lock` or a manual NX poll loop (`copilot:msg_append:{session_id}`) — those were removed. Do NOT flag the `invalidate_session_cache` call on ...
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/blocks/autopilot.py:631-638
Timestamp: 2026-04-14T07:35:09.273Z
Learning: In `autogpt_platform/backend/backend/copilot/executor/utils.py`, `CoPilotExecutionEntry` includes a `permissions: CopilotPermissions | None` field (added in PR `#12773` / commit a0184c87b9). `enqueue_copilot_turn` accepts and serializes this field into the queue entry, `_enqueue_for_recovery` in `autopilot.py` accepts and forwards `permissions` to `enqueue_copilot_turn`, and `_execute_async` in `processor.py` restores `entry.permissions` and passes it into `stream_chat_completion_sdk`/`stream_chat_completion_baseline` via `set_execution_context`. This ensures recovered sub-agent turns respect the same tool/block permission ceiling as the original in-process execution (mirroring `_merge_inherited_permissions`). Do NOT flag recovered turns as losing their permission ceiling — it is now fully propagated through the queue.
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
📚 Learning: 2026-04-16T12:33:42.787Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12796
File: autogpt_platform/backend/backend/api/features/chat/routes.py:504-527
Timestamp: 2026-04-16T12:33:42.787Z
Learning: In `autogpt_platform/backend/backend/api/features/chat/routes.py`, `get_session` (PR `#12796`, commit 3771bfad9c1) closes the TOCTOU race between the initial `stream_registry.get_active_session()` pre-check and `get_chat_messages_paginated()` with a post-check re-verification: after the DB fetch, if `is_initial_load and active_session is not None`, it calls `get_active_session` a second time; if `post_active is None` (stream completed during the window), it resets `from_start=True`, `forward_paginated=True`, and re-fetches messages from sequence 0. Do NOT flag the double `get_active_session` call pattern as redundant — it is the intentional TOCTOU mitigation for pagination direction selection.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-11T08:40:59.673Z
Learnt from: kcze
Repo: Significant-Gravitas/AutoGPT PR: 12328
File: autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts:49-61
Timestamp: 2026-03-11T08:40:59.673Z
Learning: In `autogpt_platform/frontend/src/app/(platform)/copilot/useLoadMoreMessages.ts`, clearing `olderMessages` (and resetting `oldestSequence`/`hasMore`) when `initialOldestSequence` shifts on the same session is intentional. Pages already fetched were based on a now-stale cursor; retaining them risks sequence gaps or duplicates. `ScrollPreserver` keeps the currently visible viewport intact, so only unvisited older pages are dropped. This is a deliberate safe-refetch design tradeoff.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Mock at boundaries — mock where the symbol is **used**, not where it's **defined**; after refactoring, update mock targets to match new module paths
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-04-14T14:36:22.396Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/copilot/pending_messages.py:52-64
Timestamp: 2026-04-14T14:36:22.396Z
Learning: In `autogpt_platform/backend/backend/copilot` (PR `#12773`, commit d7bced0c6): when draining pending messages into `session.messages`, each message's text is sanitized via `strip_user_context_tags` before persistence to prevent user-controlled `<user_context>` injection from bypassing the trusted server-side context prefix. Additionally, if `upsert_chat_session` fails after draining, the drained `PendingMessage` objects are requeued back to Redis to avoid silent message loss. Do NOT flag the drain-then-requeue pattern as redundant — it is the intentional failure-resilience strategy for the pending buffer.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-26T07:00:03.405Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12574
File: autogpt_platform/backend/backend/copilot/sdk/transcript.py:980-990
Timestamp: 2026-03-26T07:00:03.405Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/transcript.py`, `_rechain_tail` intentionally rewrites `parentUuid` for **all** tail entries (not just the first), because a single assistant turn can span multiple consecutive JSONL entries sharing the same `message.id` (e.g., a thinking entry + a tool_use entry). Their original `parentUuid` values may reference entries that were absorbed into the compressed prefix, so sequential rechaining of the entire tail is required to maintain a valid parent→child graph. The test `test_chains_multiple_tail_entries` validates this: the second tail entry's `parentUuid` is rewritten from its original value to the uuid of the first tail entry.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-04-15T13:44:31.808Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12797
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1991-2021
Timestamp: 2026-04-15T13:44:31.808Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py` (`_run_stream_attempt`), the pre-create block (PR `#12797`) intentionally does NOT call `state.transcript_builder.append_assistant(...)` when inserting the empty assistant placeholder into `ctx.session.messages`. The transcript is left ending at the `tool_result` entry (N entries) while `message_count` metadata is N+1. This mismatch is benign and deliberate: on the next `--resume`, the SDK sees the transcript ending at `tool_result` and correctly regenerates the assistant response. Pre-appending the assistant turn to the transcript would suppress regeneration while leaving `session.messages[-1].content = ""` permanently (worse outcome). On the gap-fallback path, `transcript_msg_count (N+1) >= msg_count-1 (N)` means no gap is injected for the empty placeholder, which is correct because injecting an empty assistant message as context would mislead the SDK. Do NOT flag this transcript/message_count discrepancy as a bug.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/**/test/**/*.py : Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-17T10:57:12.953Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/copilot/db_test.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/copilot/db_test.py (1)
297-303: LGTM — good coverage of user_id propagation through both expansion calls.Iterating
call_args_listand asserting both boundary and visibility queries carry theSession.is.userIdfilter is exactly the right defense against a cross-tenant leak regression in the expansion helpers.
majdyz
enabled auto-merge
April 17, 2026 11:56
majdyz
disabled auto-merge
April 17, 2026 12:08
majdyz
enabled auto-merge
April 17, 2026 12:08
majdyz
disabled auto-merge
April 17, 2026 12:23
github-project-automation
Bot
moved this from 🆕 Needs initial review
to ✅ Done
in AutoGPT development kanban
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
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.
Why / What / How
Why: PR #12796 changed completed copilot sessions to load messages from sequence 0 forward (ascending), which broke the standard chat UX — users now land at the beginning of the conversation instead of the most recent messages. Reported in Discord.
What: Reverts the forward pagination approach and replaces it with a visibility guarantee that ensures every page contains at least one user/assistant message.
How:
Changes
Checklist