feat(platform): retire brain dump greeting once a user has a session - #13804
Conversation
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
github-actions
Bot
added
platform/frontend
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe onboarding flow shares provider discovery, generates greetings concurrently with understanding extraction, reports explicit pending state, and updates loader, greeting transition, provider fallback, and recommendation timeout behavior. ChangesOnboarding flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant OnboardingService
participant generate_intro
participant IntroCard
Client->>OnboardingService: Request onboarding intro
OnboardingService->>generate_intro: Start greeting generation
generate_intro-->>OnboardingService: Greeting or pending state
OnboardingService-->>Client: IntroCardResponse
Client->>IntroCard: Render loader or greeting
IntroCard-->>Client: Show animated greeting
Possibly related PRs
Suggested reviewers: Poem
✨ Finishing Touches 💡 1📝 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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts (1)
98-109: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDeduplicate personalized provider IDs before rendering.
This mapping preserves repeated provider IDs.
ConnectToolsPanelthen renders duplicate cards with the sameprovider.idReact key. Keep only the first valid recommendation for each provider ID. Add a test with repeated recommendation IDs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts around lines 98 - 109, Update the personalized provider construction in useConnectToolsPanel to deduplicate recommendation.provider values before mapping, retaining the first valid recommendation for each provider ID. Ensure ConnectToolsPanel receives at most one entry per provider.id, and add a test covering repeated recommendation IDs.
🧹 Nitpick comments (2)
autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
load_all_blocksto module scope.Line 26 uses a local import for a required backend module. Import
load_all_blockswith the other module imports, then call it insideknown_providers.Proposed change
+from backend.blocks import load_all_blocks from backend.api.features.integrations.models import ( get_all_provider_names, get_provider_description, ) def known_providers() -> dict[str, str | None]: ... try: - from backend.blocks import load_all_blocks - load_all_blocks()As per coding guidelines, "Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies like
openpyxl."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py` around lines 26 - 28, Move the load_all_blocks import from inside known_providers to the module-level import section, then retain the load_all_blocks() call within known_providers.Source: Coding guidelines
autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx (1)
1-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace
GreetingLoaderin its component directory.Move this new component to
components/GreetingLoader/GreetingLoader.tsx. Keep related hooks and helpers in the same directory when they are added. As per coding guidelines, “Structure components asComponentName/ComponentName.tsx+useComponentName.ts+helpers.ts.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx around lines 1 - 15, Move the GreetingLoader component from the EmptySession components location into a dedicated components/GreetingLoader/GreetingLoader.tsx directory, preserving its exported API and updating all imports to the new path. Keep any future GreetingLoader-specific hook or helper files alongside the component in that directory.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx:
- Around line 19-22: Add an accessible loading status to the greeting loader
container around data-testid="greeting-loader", such as an appropriate role and
screen-reader-only loading text, while keeping GlassOrb decorative and hidden
from assistive technology.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/OnboardingIntroCard/useOnboardingIntroCard.ts:
- Around line 165-170: Update the isVisible calculation in
useOnboardingIntroCard to also require !serverSaysDone, preventing completed
greetings from rendering before the post-render state update. Add a test
covering greeting_done: true with a nonempty greeting and assert the intro card
remains hidden.
---
Outside diff comments:
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts:
- Around line 98-109: Update the personalized provider construction in
useConnectToolsPanel to deduplicate recommendation.provider values before
mapping, retaining the first valid recommendation for each provider ID. Ensure
ConnectToolsPanel receives at most one entry per provider.id, and add a test
covering repeated recommendation IDs.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py`:
- Around line 26-28: Move the load_all_blocks import from inside known_providers
to the module-level import section, then retain the load_all_blocks() call
within known_providers.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx:
- Around line 1-15: Move the GreetingLoader component from the EmptySession
components location into a dedicated
components/GreetingLoader/GreetingLoader.tsx directory, preserving its exported
API and updating all imports to the new path. Keep any future
GreetingLoader-specific hook or helper files alongside the component in that
directory.
🪄 Autofix
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 Plus
Run ID: a2849bc0-a196-49dc-a8b1-24ad0f4ae309
📒 Files selected for processing (20)
autogpt_platform/backend/backend/api/features/onboarding_dump/intro.pyautogpt_platform/backend/backend/api/features/onboarding_dump/intro_test.pyautogpt_platform/backend/backend/api/features/onboarding_dump/models.pyautogpt_platform/backend/backend/api/features/onboarding_dump/providers.pyautogpt_platform/backend/backend/api/features/onboarding_dump/recommend.pyautogpt_platform/backend/backend/api/features/onboarding_dump/service.pyautogpt_platform/backend/backend/api/features/onboarding_dump/service_test.pyautogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/usePreparingStep.test.tsautogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/usePreparingStep.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/EmptyHero.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/GreetingLoader.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/OnboardingIntroCard.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/OnboardingIntroCard.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/useOnboardingIntroCard.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/useOnboardingIntroCard.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/ConnectToolsPanel.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.tsautogpt_platform/frontend/src/app/api/openapi.json
Withhold the onboarding greeting from anyone with an existing chat session, ground both LLM jobs in the live provider registry, and swap the hero stand-in for a centered orb that flies into the intro card.
6007510 to
fe5d69a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13804 +/- ##
========================================
Coverage 77.92% 77.93%
========================================
Files 2906 2913 +7
Lines 219310 219520 +210
Branches 20803 20809 +6
========================================
+ Hits 170906 171072 +166
- Misses 43808 43840 +32
- Partials 4596 4608 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…he intro card on the server verdict
|
Addressed the two out-of-line findings from the CodeRabbit review in 5d36ec4:
The three failing |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
The intro route now asks how many chat sessions the user has, which is a real Prisma query. Left unstubbed the TestClient requests reached the database on their own event loop, and the connection left behind in the shared pool outlived that loop — breaking the next test that queried for real with "Event loop is closed".
… card The session count is the verdict; the flag only saves paying for the count again. Leaving the write outside the guard let a transient database error surface as a 500 on the card it was retiring.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py (1)
607-613: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNarrow the greetingSeen write handler to the onboarding DB exception contract.
The failed-write behavior and regression test are covered, but
except Exceptionhere still catches non-DB programming errors while leaving otherdb.*()callers unchecked. Wrap themark_greeting_seenupsert as a boundary exception, or catch theonboarding_dump.dbwrite failures directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/onboarding_dump/service.py` around lines 607 - 613, The mark_greeting_seen error handler currently catches all exceptions; narrow it to the onboarding_dump DB exception contract. Update the try/except around db.mark_greeting_seen to catch the repository’s specific database/write exception, preserving the warning and non-500 behavior while allowing programming errors to propagate.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/onboarding_dump/service.py`:
- Around line 607-613: The mark_greeting_seen error handler currently catches
all exceptions; narrow it to the onboarding_dump DB exception contract. Update
the try/except around db.mark_greeting_seen to catch the repository’s specific
database/write exception, preserving the warning and non-500 behavior while
allowing programming errors to propagate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6056781e-97c8-4739-80b5-4bb24252d4fa
📒 Files selected for processing (2)
autogpt_platform/backend/backend/api/features/onboarding_dump/service.pyautogpt_platform/backend/backend/api/features/onboarding_dump/service_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- autogpt_platform/backend/backend/api/features/onboarding_dump/service_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: lint
- GitHub Check: integration_test
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: end-to-end tests
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: lint
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.13)
- GitHub Check: Check PR Status
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.12)
- GitHub Check: lint
- GitHub Check: types
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (4)
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/api/features/onboarding_dump/service.py
autogpt_platform/backend/backend/api/features/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update routes in '/backend/backend/api/features/' and add/update Pydantic models in the same directory for API development
Files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
autogpt_platform/backend/**/api/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/api/**/*.py: UseSecurity()instead ofDepends()for authentication dependencies to get proper OpenAPI security specification
Follow SSE (Server-Sent Events) protocol: usedata:lines for frontend-parsed events (must match Zod schema) and: commentlines for heartbeats/status
Files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
🧠 Learnings (11)
📚 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/api/features/onboarding_dump/service.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/api/features/onboarding_dump/service.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/api/features/onboarding_dump/service.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/api/features/onboarding_dump/service.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/api/features/onboarding_dump/service.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).
Applied to files:
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
🪛 Ruff (0.16.1)
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py
[warning] 612-612: Do not catch blind exception: Exception
(BLE001)
|
On the latest CodeRabbit nitpick (narrow the
The write is bookkeeping behind a verdict that already stands, so swallowing broadly and logging is the intended behaviour here. |
|
/review |
![autogpt-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Automated Review — PR #13804
PR #13804 — feat(platform): retire brain dump greeting once a user has a session
Author: Abhi1992002 | Files: 21
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the description explains the motivation (retire the greeting once a user has a real session), the mechanism (session-count gate, explicit greeting_pending flag, shared provider registry, concurrent greeting/extraction), and the UX rework (orb layoutId flight).
What This PR Does
Previously the onboarding "brain dump" greeting could reappear for users who already had a chat session, and the UI faked a "still generating" state with a duplicated hero component. This PR gates the greeting on whether the user has any existing session (_has_chatted → get_user_session_count), replaces the overloaded empty-string "still generating" signal with an explicit greeting_pending boolean, and shares one provider registry (providers.py) between the greeting and recommender LLM jobs so the model can only name integrations that actually exist. It also runs greeting generation and understanding-extraction concurrently (asyncio.gather), retires a dual-render hero in favor of a single orb that animates into the intro-card heading, and adds a "Popular places to start" fallback for connect-tools.
Specialist Findings
🛡️ Security ✅ — No injection/authz/secret issues. All new data access keys off the authenticated user_id; the greeting render path is XSS-safe (escaped React children, no dangerouslySetInnerHTML).
🔵 Informational: user transcript is concatenated into the greeting LLM prompt (intro.py:147) where the "only name real tools" constraint is advisory — but blast radius is self-contained (greeting shown only to the same user; recommendations separately validated against the known registry in recommend.py).
🏗️ Architecture ✅ — Net structural improvement: retires dual-render hero debt, consolidates the provider registry into one shared module, and replaces the implicit empty-string protocol with an explicit flag. Coupling went down (EmptyHero shed its OnboardingIntroCard imports).
🟠 _has_chatted runs a full COUNT(*) for a presence check on a polled path (service.py:596).
⚡ Performance ✅/asyncio.gather concurrency is a genuine latency win. Two efficiency concerns: an added DB round-trip on a 1.5s-polled endpoint, and load_all_blocks() running synchronously inside async LLM jobs. Neither is a correctness/data-loss issue.
🧪 Testing ✅/_has_chatted branch matrix, greeting_pending server+client tests, dedup/fallback tests. Gaps: the asyncio.gather failure-isolation assumption (service.py:434, called without return_exceptions=True) and the new providers.py module have no direct tests.
📖 Quality ✅ — Clean refactor, excellent docstrings, thorough dead-code cleanup.
🔵 _has_chatted is a bool predicate that also writes (db.mark_greeting_seen) — the name hides the mutation; a few comments narrate change-history rather than the standing contract.
📦 Product ✅/prefers-reduced-motion.
📬 Discussion ✅ — All 5 bot review threads (CodeRabbit, Sentry) addressed with cited fix commits + covering tests, and the one declined nit has a sound rationale. No human approval yet (REVIEW_REQUIRED); manual test-plan checkboxes unchecked.
🔎 QA ✅ — All 10 backend scenarios verified live via API + DB with before/after evidence: session-based retirement, greeting_pending, greetingSeen idempotency, session-count fail-safe, shared 59-provider registry in the prompt, plus negative auth (401 for missing/garbage token). EmptyHero refactor renders correctly end-to-end. Frontend animation surfaces couldn't be driven in-browser (gated behind a pre-existing, unrelated onboarding paywall) but are covered by shipped Vitest suites that GitHub CI runs.
🟠 Should Fix
- Full COUNT on a 1.5s-polled endpoint (
service.py:547/:596) —_has_chattedcallsget_user_session_count(aCOUNT(*)overChatSession) on everyget_intro_cardpoll; for a new user (count 0) it never short-circuits, so each of ~10 onboarding polls fires a count-all query. Reorder so the pending-status path wins first (a pending user is definitionally new), gate onstatus == completed, or use anEXISTS/LIMIT 1presence check. (Flagged by: performance, architect — 2 specialists) - Orb flight animation ignores
prefers-reduced-motion(GreetingLoader.tsx:24,OnboardingIntroCard.tsx:188) — only the inner pulse is gated behinduseReducedMotion(); the shared-layoutIdspring (ORB_FLIP_TRANSITION) fires unconditionally, so a reduced-motion user still sees the orb spring across the page.useReducedMotionis already imported — gate the transition or wrap in<MotionConfig reducedMotion="user">. (Flagged by: product) asyncio.gatherfailure path untested (service.py:434) — gather is used withoutreturn_exceptions=True; a comment asserts neither task can lose the other to a failure, but nothing tests that a raise ingenerate_intro(or_extract_understanding) still completes the sibling. Add a test that forces each to raise, or passreturn_exceptions=Trueand handle results explicitly. (Flagged by: testing)- New
providers.pyshipped without tests (providers.py:28) — theknown_providers()load_all_blocksfailure branch (log + static fallback) is the graceful-degradation both LLM jobs depend on, yet is never exercised. Add aproviders_test.pypatchingload_all_blocksto raise. (Flagged by: testing) - Predicate
_has_chattedperforms a hidden write (service.py:588) — abool-returning predicate also callsdb.mark_greeting_seen; callers ofif await _has_chatted(...)won't expect a mutation. Rename (e.g._retire_greeting_if_chatted) or split the read from the bookkeeping write. (Flagged by: quality)
🟡 Nice to Have
- Memoize
load_all_blocks()(providers.py:24) — unmemoized and now called by both LLM jobs; a single finalize can load all blocks twice, and a cold synchronous call on the shared event loop stalls concurrent requests. Confirm it's cached, or wrap inasyncio.to_thread. (performance, architect) - Make the 15s recommendation ceiling configurable (
usePreparingStep.ts:30) — the 60s→15s cut is defensible with the faster model, but a job answering at ~18s under load is silently cut to generic providers; consider ~20-25s or env-configurable. (product) - Trim/confirm caching of the registry prompt prefix (
intro.py:147) — the full registry is prepended to every greeting generation; verify prompt caching amortizes the static prefix. (performance) - Extract shared orb constants into a dedicated module (
GreetingLoader.tsx:5) — importingGREETING_ORB_LAYOUT_ID/ORB_FLIP_TRANSITION/SMALL_ORB_PARAMSfrom a sibling component couples the two; ahelpers/greetingOrb.tswould be cleaner. (quality, architect)
🔵 Nits
- Comment durability (
models.py:117,EmptyHero.tsx:11,OnboardingIntroCard.tsx:108) — several comments narrate the old protocol ("used to carry…", "no longer arrives…"); rewrite to the standing contract. (architect) - Braceless single-line
if(useOnboardingIntroCard.ts:114) — inconsistent with surrounding braced ifs. (quality) fallbackProvidershelper placement (useConnectToolsPanel.ts:128) — defined after its call site inside the hook; move tohelpers.tsper AGENTS.md. (quality)
QA Screenshots
| Screenshot | Description |
|---|---|
 |
/copilot for a user with a session: greeting retired, regular centered EmptyHero, no intro-card/orb, chat composer present ✅ |
Human Review Needed
NO — This is routine feature work on onboarding UX. New data access keys off the authenticated user_id with no new endpoints, no changes to the authentication/authorization boundary, and no credential/secret handling. QA verified the auth boundary (401s) live.
Risk Assessment
Merge risk: LOW | Rollback: EASY (isolated feature; no schema migration in the security/data boundary, changes are additive and behind onboarding flow)
CI Status
GitHub CI (authoritative): ✅ All green per discussion review — 44+ checks including test 3.11/3.12/3.13, type-check, lint, CodeQL, end-to-end, integration, and check-API-types on head SHA a4946c0; MERGEABLE, patch coverage 94.9%.
Local harness: lint (frontend + backend), typecheck, and build passed; pnpm test:unit failed locally. Since GitHub CI ran the same frontend suite green on this head SHA, the local failure is environment skew, not a code defect — reported as a warning only.
UI Testing — Variant Results
✅ local: All backend behaviors (greeting retirement on existing session, greeting_pending flag, greetingSeen idempotency, shared provider registry) verified live via API+DB with negative auth tests; EmptyHero refactor renders correctly, no defects found.
✅ hosted: Greeting retirement on existing session, greeting_pending signaling, GreetingLoader orb, and provider-registry fallback all verified working live via API, DB, and browser; no defects found.
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
github-actions
Bot
added
the
conflicts
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
github-actions
Bot
removed
the
conflicts
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
…ced motion Backend: - get_intro_card answers the pending case before the session lookup, so the 1.5s poll no longer re-runs it every cycle; the path-B and pending conditions move into named predicates. - _has_chatted -> _retire_greeting_if_chatted, naming the write it does. - New copilot/db.user_has_any_session (SELECT 1 ... LIMIT 1) replaces the full COUNT(*) that was only compared against zero. - _extract_understanding now guards its whole body, so the gather with generate_intro cannot lose one result to the other's failure; three tests hold both halves to that contract. - providers_test.py covers the registry's block-load failure branch and provider_lines for described/undescribed/empty registries. - greeting_pending and the gather comment state the standing contract instead of the change history. Frontend: - Shared orb constants move to copilot/helpers/greetingOrb.ts, so GreetingLoader no longer imports OnboardingIntroCard's internals. - The shared-layoutId flight respects prefers-reduced-motion in both components, matching the pulse that already did. - fallbackProviders moves into OnboardingWelcomeDialog/helpers.ts. - Tests for the fallback padding path, the query-error branch, the flag-off branch, and GreetingLoader's status announcement.
|
/review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
📋 Automated Review — PR #13804
PR #13804 — feat(platform): retire brain dump greeting once a user has a session
Author: Abhi1992002 | Files: 27
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the description explains the trust bug (greeting shown to users mid-conversation and promising tools the platform can't run), the change (session-gated retirement + explicit greeting_pending flag + shared provider registry), and the mechanism. One gap: the author's manual test checklist is entirely unchecked (see Discussion).
What This PR Does
Previously the onboarding "brain dump" greeting kept showing even to users who already had a chat session, and it could promise integrations the platform can't actually run. This PR retires the greeting once a user has any real (non-dream) chat session, grounds both LLM jobs (greeting + recommender) in one shared live provider registry so promised automations are real, and replaces the implicit "empty string means still generating" protocol with an explicit greeting_pending boolean. It also swaps the fragile dual-hero rendering for a dedicated GreetingLoader whose orb flies into the heading via a shared layoutId, and adds a "Popular places to start" fallback when the recommender returns nothing.
Specialist Findings
🛡️ Security ✅ — The new user_has_any_session raw SQL is properly parameterized ($1 bind, static exclusion constant — no user string reaches SQL text; copilot/db.py:697). The /intro route derives user_id from the JWT via Security(get_user_id) and is gated fail-closed behind require_brain_dump_flag — no IDOR surface. LLM prompt context is server-controlled; no new prompt-injection surface introduced.
🏗️ Architecture ✅ — Genuine debt reduction: providers.py de-duplicates the provider registry both LLM jobs consume, and get_intro_card decomposes into named predicates (_nothing_to_reflect, _greeting_still_writing, _retire_greeting_if_chatted). Notes a defensible new cross-feature edge (onboarding_dump/service.py → copilot.db) — fine for one consumer, promote to a shared data/ layer if a third appears.
🟡 FALLBACK_PROVIDER_IDS hardcodes six provider ids in a component helper (self-heals from the live registry, so degrades gracefully).
⚡ Performance user_has_any_session (SELECT 1 … LIMIT 1) and the pending case is answered before the session lookup (QA confirmed greetingSeen stays f and the session table isn't touched while pending). Two residual concerns remain (below).
🟠 The session read + mark_greeting_seen write run before the cheap Path-B / dump is None checks, so skipped-onboarding users still pay a round-trip per intro-card fetch (service.py:559).
🧪 Testing asyncio.gather sides degrade independently, empty-registry prompt, card-flash regression, 15s ceiling). One real gap remains.
🟠 user_has_any_session is mocked in every consumer and has no direct test, despite db_session_listing_test.py establishing that convention for sibling session queries (copilot/db.py:690).
📖 Quality ✅ — Clean, well-decomposed, A- readability. Clear naming throughout; greeting_pending is a good rename over the old inference. Two nits: a side-effecting mutation inside .map() (useConnectToolsPanel.ts:88) and a duplicated has_session test fixture.
📦 Product ✅ — All five PR claims implemented and backed by tests; graceful degradation on every failure path. Accessibility is strong (role="status" + sr-only label, reduced-motion respected via ORB_FLIP_TRANSITION_REDUCED). Fallback heading correctly distinguishes "Recommended from our conversation" vs "Popular places to start" so generic picks are never mislabeled.
📬 Discussion BLOCKED on REVIEW_REQUIRED — only bots have reviewed — and the author's manual-test checklist is entirely unchecked.
🔎 QA ✅ — Backend behavior exercised end-to-end against the live stack (fresh user, direct DB manipulation). Headline change confirmed: no session → path:B, greeting_done:false; insert one real ChatSession → path:A, greeting_done:true, greeting:"" with greetingSeen=t (paid once). Dream exclusion verified (dream-only session → not retired), greeting_pending precedence verified (pending wins, session lookup skipped), unauth intro → 401. Could not reach the in-browser /copilot greeting UI — a fresh Better Auth user has no org context and is redirected to a plan-selection screen (environment limitation, not a PR defect); compensated by running the touched frontend suites directly (10 passed).
🟠 Should Fix
- Direct test for
user_has_any_session(autogpt_platform/backend/backend/copilot/db.py:690) — the new raw SQL is mocked everywhere it's consumed and never exercised; a filter/param regression would silently suppress greetings for every new user (errors are swallowed →False). Add a mocked-query_raw_with_schematest matchingdb_session_listing_test.py: assert the null-safe dream filter,LIMIT 1,(user_id,)binding,Truefor a non-empty row set andFalsefor[]. (Flagged by: testing, QA — 2 specialists) - Session lookup + write ordered before Path-B/skip checks (
autogpt_platform/backend/backend/api/features/onboarding_dump/service.py:559) — reorder so obviously-Path-B /dump is Noneresponses return before theuser_has_any_sessionread andmark_greeting_seenwrite, so skipped-onboarding users don't pay a round-trip per intro-card load. (Flagged by: performance, architect — 2 specialists) - Confirm the
ChatSession.userIdindex (autogpt_platform/backend/backend/copilot/db.py:690) — theLIMIT 1win evaporates into a sequential scan under load without it. Verifyschema.prismahas an index onChatSession(userId); add one if absent. (Flagged by: performance — 1 specialist)
🟡 Nice to Have
- Warm
load_all_blocks()at startup (autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py:27) — synchronous@cachedblock load inside the async greeting coroutine stalls the event loop on the first post-restart request, negating theasyncio.gatherconcurrency win for that one cold call. (performance) - Source the "popular" fallback list from config (
OnboardingWelcomeDialog/helpers.ts:6) —FALLBACK_PROVIDER_IDSwill silently rot; low risk given the registry-padding fallback. (architect) - Make the 15s recommendation ceiling configurable (
usePreparingStep.ts:30) — a job landing between 15–60s now yields the generic fallback; watchclaude-haiku-4-5p95 post-merge, or makeRECOMMENDATIONS_MAX_WAIT_MSenv-configurable. Kept-by-design by the author. (performance, product, discussion — 3 specialists) - Greeting prose is unvalidated LLM output (
intro.py:147) — id validation covers the recommendation tiles, not the free-form greeting text; low risk, worth documenting the residual surface. (discussion)
🔵 Nits
- Change-relative comment (
usePreparingStep.ts:30) — "It runs on the fast model now" dates a transition; restate as a standing fact. (architect) - Mutation inside
.map()(useConnectToolsPanel.ts:88) — dedup-then-map orreducekeeps the transform pure. (quality) - Duplicated
has_sessionfixture (service_test.py:171/routes_test.py:184) — hoist to a sharedconftest.py. (quality) - A11y information asymmetry (
GreetingLoader.tsx:24) — sighted users see a bare orb; consider a subtle visible caption or confirm the orb-only treatment with design. (product)
QA Screenshots
| Screenshot | Description |
|---|---|
 |
Authenticated frontend session; the /copilot greeting UI itself was unreachable (fresh user lacks org context, redirected to onboarding) — backend paths verified via live API/DB instead |
Human Review Needed
NO — This is routine onboarding/feature work. The one new data-layer query is a read scoped to the caller's own ChatSession rows (no auth/permission boundary change), authorization is JWT-derived and unchanged, and no secrets or trust boundaries are touched. GitHub's REVIEW_REQUIRED gate is a repo policy matter, not a signal that this review missed something.
Risk Assessment
Merge risk: LOW | Rollback: EASY — additive, defaulted (greeting_pending: false) API field; every new failure path degrades to "still greet the user"; no schema-destructive migration.
CI Status
Local harness (review sandbox): lint ✅ (frontend + backend), typecheck ✅, build ✅; pnpm test:unit ❌. GitHub CI: reported green (44/44) in the discussion thread — per repo-authoritative CI, the local test:unit failure is environment skew (the review sandbox runs the frontend from a production build without test files, as QA noted), not a code defect. Treat as a warning, not a blocker.
UI Testing — Variant Results
✅ local: All backend greeting-retirement, greeting_pending, dream-exclusion, and provider-registry behaviors verified live (Actual=Expected); backend (24) and touched frontend (10) tests pass; no defects found in the diff.
✅ hosted: Live API + DB testing confirms greeting retirement on existing session, dream-session exclusion, the new greeting_pending flag, and the shared provider registry; 79 backend and 62 frontend touched tests pass and negative auth returns 401.
Addresses the review on #13804. - `user_has_any_session` was mocked at every call site and never exercised; a regression in the dream filter or the LIMIT 1 is silent because the caller swallows failures and answers "no". Two direct tests in `db_session_listing_test.py` now hold the query shape and both result branches. - The near-identical `has_session` fixture in `service_test.py` and `routes_test.py` moves to a package `conftest.py`, keeping the event-loop warning in one place. - Dedup the recommended providers before mapping so the `.map` callback no longer mutates an outer Set. - Document that greeting prose is unvalidated model output, and restate the 15s-ceiling comment as a standing fact rather than a transition.
|
Addressed the automated review at Fixed
Already satisfied — Declining — reordering the session lookup after the Path-B checks The cost it's trying to save also isn't per-load. Also declining, with reasoning in-thread: startup warm of Local: 79 backend + 49 frontend tests pass; backend format/lint and frontend Note the remaining gate is |
|
/review |
There was a problem hiding this comment.
📋 Automated Review — PR #13804
PR #13804 — feat(platform): retire brain dump greeting once a user has a session
Author: Abhi1992002 | Files: 28
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the description explains the greeting-retirement motivation, the greeting_pending protocol change, and the shared-registry/orb refactors. [ ]) — QA and discussion both confirmed the automated suite is comprehensive and green, but the author has not self-attested the manual orb-animation flow.
What This PR Does
Previously the onboarding "brain dump" greeting could reappear for users who had already started chatting, and the client couldn't tell "greeting still generating" from "no greeting coming." This PR retires the greeting once a user has any real chat session (presence-checked via a LIMIT 1 query that preserves dream-session exclusion), adds an explicit greeting_pending flag to disambiguate the mid-pipeline state, grounds both LLM jobs in one shared provider registry, and replaces a fragile dual-hero render hack with a single orb that animates into the intro-card heading via a shared layoutId.
Specialist Findings
🛡️ Security ✅ — The new raw SQL in user_has_any_session (copilot/db.py:698) is fully parameterized ("userId" = $1), authorization is scoped to the caller on every new path, and no new PII exposure was introduced. The only surface is the user transcript flowing into the greeting LLM prompt (intro.py:156), which is documented, self-scoped, and mitigated by validating connectable provider IDs against known_providers().
🔵 Prompt-injection reaches only the injecting user's own greeting prose — no change required.
🏗️ Architecture ✅ — Genuine cleanup: the provider registry (providers.py) and orb constants (greetingOrb.ts) are extracted in the correct dependency direction, get_intro_card is decomposed into named predicates, and the API change is backward-compatible (greeting_pending defaults False). Verified the new onboarding_dump → copilot.db import is non-circular.
🟡 FALLBACK_PROVIDER_IDS (helpers.ts:7) hardcodes a curated list that degrades silently if an ID is renamed.
⚡ Performance ✅ — Net win: asyncio.gather runs extraction + greeting concurrently, recommender moved to claude-haiku-4-5, give-up ceiling dropped 60s→15s, and presence is now SELECT 1 ... LIMIT 1 instead of a full COUNT (prior critical finding, confirmed fixed). One residual: known_providers() calls synchronous load_all_blocks() on the async greeting path (providers.py:27), which can block the event loop on the first cold post-restart request.
🟠 Cold load_all_blocks() on the gathered async path partially defeats the concurrency win (Flagged by: performance, quality — 2 specialists).
🧪 Testing asyncio.gather non-raising contract, session-lookup failure, seen-write failure, and pending short-circuit each have dedicated tests, plus db_session_listing_test.py asserts LIMIT 1 + dream filter + param binding. Frontend gaps remain on view wiring and behavioral constants.
🟠 The EmptySession loader-vs-hero swap (the PR's centerpiece) and the new 15s give-up ceiling lack direct assertions.
📖 Quality ✅ — Readability grade A: precise naming, why-not-what docstrings, intent-revealing test names. Comments are dense in the animation files but earn their place given the layoutId handoff subtlety. Minor: _nothing_to_reflect(dump) is evaluated twice per request (service.py).
📦 Product ✅ — All PR claims implemented and backed by tests. Fallback provider IDs all exist in the enum; degradation paths keep the user's greeting rather than 500ing; "Popular places to start" copy honestly stops claiming personalization. Note: greeting suggested-prompts are not registry-validated (only the recommend.py tiles are), and haiku p95 should be confirmed < 15s so recs don't silently fall back.
📬 Discussion ✅ — All 39 review threads resolved with traceable commits; author engaged substantively with coderabbit/sentry/autogpt-pr-reviewer feedback (accessible loader text, one-paint flash guard, dup-key fix, LIMIT 1 refactor, wrapped seen-write). No open threads. Gaps: no human approval yet (REVIEW_REQUIRED) and the unchecked manual checklist.
🔎 QA ✅ — Verified live end-to-end against the running stack: Path B for fresh users, greeting_pending:true for a mid-pipeline dump, retirement flipping greetingSeen f→t when a session exists (durable across reload), the intro-card orb rendering in the heading, the regular hero returning post-retirement, and 401s on missing/garbage tokens. Only the connect-tools "Popular places to start" fallback was verified via unit tests rather than click-through.
🟠 Should Fix
- Cold
load_all_blocks()blocks the gathered async path (backend/backend/api/features/onboarding_dump/providers.py:27) —known_providers()runs a synchronous block-import sweep beforegenerate_intro's firstawait; on the first request after a process/executor restart it stalls the concurrent extraction coroutine and every other request on the worker, undercutting theasyncio.gatherlatency win. Wrap inasyncio.to_thread(load_all_blocks)or pre-warm at startup. (Flagged by: performance, quality — 2 specialists) - Loader-vs-hero swap has no integration test (
frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsx:134) — the headline UX (renderGreetingLoaderwhileisAwaitingGreeting,EmptyHerootherwise) is only tested with the loader in isolation; an inverted/dropped condition would ship green. Add anEmptySessionintegration test covering both branches. (Flagged by: testing) - 15s give-up ceiling change is unasserted (
frontend/src/app/(no-navbar)/onboarding/steps/__tests__/usePreparingStep.test.ts:13) — the modified tests deliberately stay short of the ceiling, so nothing verifies the user advances at 15s when the job never resolves. Add a test that advances pastRECOMMENDATIONS_MAX_WAIT_MSand assertsonCompletefires. (Flagged by: testing)
🟡 Nice to Have
- Assert
FALLBACK_PROVIDER_IDSresolve against the live registry (frontend/.../OnboardingWelcomeDialog/helpers.ts:7) — guard the curated list against silent bit-rot. (architect) - Validate greeting suggested-prompts against
known_providers(backend/.../onboarding_dump/intro.py:156) — or add telemetry so a hallucinated automation naming an unavailable tool is detectable. (product) - Memoize the assembled provider registry (
backend/.../providers.py:19) — both LLM jobs recompute the{id: description}map per completion. (performance)
🔵 Nits
- Negative seen-flag assertion for new users (
backend/.../onboarding_dump/service_test.py:821) — addgreeting_seen_writes == 0on Path A to catch premature-retirement regressions. (testing) _nothing_to_reflect(dump)evaluated twice per request (backend/.../onboarding_dump/service.py) — compute once and reuse. (quality)- Shared
GREETING_ORB_LAYOUT_IDdivergence unguarded (frontend/.../EmptySession/__tests__/greeting-loader.test.tsx:11) — a unit assertion that both components import the same constant would prevent a silent swap-instead-of-travel. (testing)
QA Screenshots
| Screenshot | Description |
|---|---|
 |
Purple orb in the "Hey, Test" heading with personalized greeting + copy-transcript button ✅ |
 |
After a session exists: greeting retired, centered EmptyHero "Tell me about your work…" ✅ |
Human Review Needed
NO — This is routine onboarding/UX product work by a maintainer. It touches no system authentication, credential storage, or cross-service trust boundary; the new raw SQL is parameterized and scoped to the caller, and QA verified the auth-scoping (401s) live. Size and the DB-query addition alone do not warrant it.
Risk Assessment
Merge risk: LOW | Rollback: EASY (backward-compatible API addition; revert is clean)
CI Status
Local harness: 4/5 checks pass — frontend lint (79s), backend lint (97s), frontend typecheck (48s), and frontend build (282s) all green; pnpm test:unit failed in the sandbox (400s). Per the discussion specialist, GitHub CI ran the frontend/backend suites — including all codecov gates — green on this head SHA, so the local test:unit failure is environment skew, not a code defect. GitHub CI: reported green by the discussion specialist (all required checks: test 3.11/3.12/3.13, e2e, integration, type-check, lint, CodeQL, codecov); treat as authoritative over the local harness.
UI Testing — Variant Results
✅ local: Greeting retirement, greeting_pending flag, intro-card orb, and regular-hero fallback all verified live end-to-end with no errors; only the connect-tools fallback was verified via tests rather than click-through.
✅ hosted: All new behaviors verified live — session retires the greeting (with greetingSeen recorded), dream sessions are excluded, greeting_pending reports mid-generation correctly, auth negatives return 401, and the changed OnboardingIntroCard renders cleanly with no server errors.
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
1 participant
Why / What / How
The brain-dump greeting could greet a user who was already mid-conversation (
greetingSeenonly covers first sends that went through the copilot home while the flag was on), the LLM promised integrations we cannot actually run, and the "still generating" state was faked by a second hero component that had to render the heading row identically to the intro card. This PR retires the greeting for anyone with an existing chat session, feeds the live provider registry into both LLM jobs, and replaces the hero stand-in with a centered orb that flies into the intro card heading under a shared framerlayoutId.Changes 🏗️
get_intro_cardnow checksget_user_session_count— one session retires the greeting (and recordsgreetingSeenso the count is paid once). A failure there answers "no" so a genuinely new user keeps their greeting.greeting_pendingflag: the pending state is stated outright instead of inferred from an empty Path A greeting, so the client can tell "still coming" from "there isn't one"; polling and the loader gate read it directly.onboarding_dump/providers.py): extracted fromrecommend.pyand now also prepended to the greeting prompt, so the automations it proposes are ones we can run. Newintro_test.pycovers prompt assembly, including an empty registry.asyncio.gather), recommendations moved toclaude-haiku-4-5, and the onboarding loading screen's give-up ceiling dropped 60s → 15s.GreetingLoaderrenders a pulsing orb alone, centered;EmptyHerois back to being just the regular hero.OnboardingIntroCardreveals its heading and sharesGREETING_ORB_LAYOUT_IDwith the loader so the orb travels rather than being swapped.openapi.jsonforgreeting_pending.Checklist 📋
For code changes:
/providers)poetry run pytest backend/api/features/onboarding_dumpandpnpm test:unitfor the touched suites