diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/conftest.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/conftest.py new file mode 100644 index 000000000000..914846b59abb --- /dev/null +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/conftest.py @@ -0,0 +1,24 @@ +"""Shared fixtures for the onboarding brain dump tests.""" + +from unittest.mock import AsyncMock + +import pytest +from pytest_mock import MockerFixture + + +@pytest.fixture(autouse=True) +def has_session(mocker: MockerFixture) -> AsyncMock: + """A brand-new user by default; the greeting is only for those. + + Stubbed for every test in the package, not just the ones that assert + on it: the intro path asks whether the user has any chat session, + which is a real query. Left unstubbed, the route tests reach the + database on the TestClient's own event loop, and the connection they + leave behind outlives that loop and breaks the next test that + queries for real. + """ + mock = AsyncMock(return_value=False) + mocker.patch( + "backend.api.features.onboarding_dump.service.user_has_any_session", new=mock + ) + return mock diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/intro.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/intro.py index 76bde960350c..5e56ac367b2b 100644 --- a/autogpt_platform/backend/backend/api/features/onboarding_dump/intro.py +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/intro.py @@ -17,6 +17,10 @@ from backend.api.features.onboarding_dump.models import SuggestedPrompt from backend.api.features.onboarding_dump.parsing import parse_response_json +from backend.api.features.onboarding_dump.providers import ( + known_providers, + provider_lines, +) from backend.util.clients import get_openai_client from backend.util.settings import Settings @@ -129,6 +133,15 @@ async def generate_intro(transcript: str) -> tuple[str, list[SuggestedPrompt]]: Never raises: a failed or malformed generation degrades to the template below rather than costing the user their greeting. + + The greeting is unvalidated model output. ``transcript`` is the user's + own words, and the "only name real integrations" constraint in the + prompt is advisory, so an injected or hallucinated tool name can reach + the prose. That surface is deliberately left open: the text is shown + only to the person who recorded it, and the ids that become + connectable or recommended tiles are validated against + :func:`providers.known_providers` in ``recommend.py`` rather than + trusted from prose. """ text = transcript.strip() if not text: @@ -140,6 +153,7 @@ async def generate_intro(transcript: str) -> tuple[str, list[SuggestedPrompt]]: return fallback_intro(text) instructions = await _fetch_langfuse_prompt() or _LOCAL_PROMPT + content = f"{_integrations_block()}{instructions}{text}" data = None # Two attempts: at temperature 0.6 an occasional generation comes back # truncated or malformed, and one retry is far cheaper than shipping @@ -149,7 +163,7 @@ async def generate_intro(transcript: str) -> tuple[str, list[SuggestedPrompt]]: response = await asyncio.wait_for( client.chat.completions.create( model=_MODEL, - messages=[{"role": "user", "content": f"{instructions}{text}"}], + messages=[{"role": "user", "content": content}], temperature=0.6, max_tokens=3000, ), @@ -181,6 +195,25 @@ async def generate_intro(transcript: str) -> tuple[str, list[SuggestedPrompt]]: return greeting.strip()[:MAX_GREETING_CHARS], prompts[:MAX_PROMPTS] +def _integrations_block() -> str: + """The live provider registry, prepended to the instructions. + + Without it the model invents the tools it promises to wire up, and the + user's first suggested automation is one we cannot run. Goes in front + of the instructions (which end with ``Transcript:``) so the whole + static half of the message stays a stable prefix. + """ + lines = provider_lines(known_providers()) + if not lines: + return "" + return ( + "These are the integrations this platform can connect to. Only " + "promise automations that these can actually carry out, and never " + "name a tool that is not on this list:\n" + f"{lines}\n\n" + ) + + async def _fetch_langfuse_prompt() -> str | None: """Fetch the greeting instructions from Langfuse. diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/intro_test.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/intro_test.py new file mode 100644 index 000000000000..e3d2092003e2 --- /dev/null +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/intro_test.py @@ -0,0 +1,82 @@ +"""Tests for the greeting generation's prompt assembly. + +The greeting promises the user specific automations, so what the model is +told this platform can connect to is part of the contract — a greeting +built without the registry offers integrations we do not have. +""" + +import json +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest +from pytest_mock import MockerFixture + +from backend.api.features.onboarding_dump import intro + +TRANSCRIPT = "I run a bakery and want the weekly order emails handled." + +GENERATION = { + "greeting": "You mentioned the weekly order emails.", + "prompts": [ + {"title": f"Automation number {i}", "prompt": "Do it.", "icon": "sparkle"} + for i in range(5) + ], +} + + +@pytest.fixture +def client(mocker: MockerFixture) -> MagicMock: + completion = SimpleNamespace( + choices=[ + SimpleNamespace(message=SimpleNamespace(content=json.dumps(GENERATION))) + ] + ) + fake = MagicMock() + fake.chat.completions.create = AsyncMock(return_value=completion) + mocker.patch( + "backend.api.features.onboarding_dump.intro.get_openai_client", + return_value=fake, + ) + mocker.patch( + "backend.api.features.onboarding_dump.intro._fetch_langfuse_prompt", + new=AsyncMock(return_value=None), + ) + return fake + + +@pytest.mark.asyncio +async def test_greeting_prompt_carries_the_provider_registry( + client: MagicMock, mocker: MockerFixture +): + mocker.patch( + "backend.api.features.onboarding_dump.intro.known_providers", + return_value={"slack": "Team chat", "github": None}, + ) + + greeting, prompts = await intro.generate_intro(TRANSCRIPT) + + content = client.chat.completions.create.await_args.kwargs["messages"][0]["content"] + assert "- slack: Team chat" in content + assert "- github" in content + # Registry first, then the instructions, then the transcript they end + # with — the static half of the message stays a stable prefix. + assert content.index("- slack") < content.index(TRANSCRIPT) + assert greeting == GENERATION["greeting"] + assert len(prompts) == 5 + + +@pytest.mark.asyncio +async def test_greeting_prompt_survives_an_empty_registry( + client: MagicMock, mocker: MockerFixture +): + mocker.patch( + "backend.api.features.onboarding_dump.intro.known_providers", + return_value={}, + ) + + greeting, _ = await intro.generate_intro(TRANSCRIPT) + + content = client.chat.completions.create.await_args.kwargs["messages"][0]["content"] + assert content.startswith(intro._LOCAL_PROMPT[:40]) + assert greeting == GENERATION["greeting"] diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/models.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/models.py index 7a44ec8c7e89..9d73a45522f8 100644 --- a/autogpt_platform/backend/backend/api/features/onboarding_dump/models.py +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/models.py @@ -112,6 +112,10 @@ class IntroCardResponse(BaseModel): greeting: str prompts: list[SuggestedPrompt] = [] greeting_done: bool = False + greeting_pending: bool = False + """True while the greeting is still being written. Both this state and + "there is no greeting" carry an empty ``greeting``, so the client needs + this flag to tell them apart and know whether to keep polling.""" transcript: str | None = None """The full transcript of the recorded dump, so the greeting page can offer a copy button. Only present on Path A while the greeting is diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py new file mode 100644 index 000000000000..cc8ceef03497 --- /dev/null +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/providers.py @@ -0,0 +1,40 @@ +"""The live integration registry, as prompt context. + +Both LLM jobs need to know what this platform can actually connect to: +the recommender picks from the list, and the greeting needs it so the +automations it proposes are ones we can really run. Shared here so the +two can never drift onto different views of the registry. +""" + +import logging + +from backend.api.features.integrations.models import ( + get_all_provider_names, + get_provider_description, +) +from backend.blocks import load_all_blocks + +logger = logging.getLogger(__name__) + + +def known_providers() -> dict[str, str | None]: + """The live provider registry as ``{id: description}``. + + Mirrors the ``/providers`` endpoint: block modules must be imported + before AutoRegistry knows about SDK-registered providers. + ``load_all_blocks`` is ``@cached``, so the two jobs that share this + helper also share one import pass rather than each paying for it. + """ + try: + load_all_blocks() + except Exception as e: # static providers still work + logger.warning("Brain dump: block load failed: %s", e) + return {name: get_provider_description(name) for name in get_all_provider_names()} + + +def provider_lines(providers: dict[str, str | None]) -> str: + """``providers`` as one ``- id: description`` line each.""" + return "\n".join( + f"- {name}: {description}" if description else f"- {name}" + for name, description in providers.items() + ) diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/providers_test.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/providers_test.py new file mode 100644 index 000000000000..6fbb931142b4 --- /dev/null +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/providers_test.py @@ -0,0 +1,47 @@ +"""Tests for the shared provider registry both LLM jobs are given. + +The recommender picks from this list and the greeting promises +automations built on it, so the graceful-degradation guarantee — a +registry that still answers when block loading fails — is what keeps a +bad import from turning either job into a wrong answer. +""" + +from pytest_mock import MockerFixture + +from backend.api.features.onboarding_dump import providers + +MODULE = "backend.api.features.onboarding_dump.providers" + + +def test_known_providers_pairs_each_id_with_its_description(mocker: MockerFixture): + mocker.patch(f"{MODULE}.load_all_blocks") + mocker.patch(f"{MODULE}.get_all_provider_names", return_value=["slack", "notion"]) + mocker.patch( + f"{MODULE}.get_provider_description", + side_effect={"slack": "Team chat", "notion": None}.get, + ) + + assert providers.known_providers() == {"slack": "Team chat", "notion": None} + + +def test_known_providers_still_answers_when_block_loading_fails(mocker: MockerFixture): + # Statically registered providers are already on the registry, so a + # broken block import costs the SDK-registered ones — not the list. + mocker.patch(f"{MODULE}.load_all_blocks", side_effect=RuntimeError("bad import")) + mocker.patch(f"{MODULE}.get_all_provider_names", return_value=["slack"]) + mocker.patch(f"{MODULE}.get_provider_description", return_value="Team chat") + + assert providers.known_providers() == {"slack": "Team chat"} + + +def test_provider_lines_omits_the_colon_for_undescribed_providers(): + lines = providers.provider_lines({"slack": "Team chat", "notion": None}) + + assert lines == "- slack: Team chat\n- notion" + + +def test_provider_lines_is_empty_for_an_empty_registry(): + # The greeting prompt drops the whole integrations block on this, so + # an empty string here is the difference between no constraint and a + # constraint that names nothing. + assert providers.provider_lines({}) == "" diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/recommend.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/recommend.py index f0c0f419c723..56a02f4a71c9 100644 --- a/autogpt_platform/backend/backend/api/features/onboarding_dump/recommend.py +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/recommend.py @@ -11,17 +11,20 @@ import logging import os -from backend.api.features.integrations.models import ( - get_all_provider_names, - get_provider_description, -) from backend.api.features.onboarding_dump.models import RecommendedProvider from backend.api.features.onboarding_dump.parsing import parse_response_json +from backend.api.features.onboarding_dump.providers import ( + known_providers, + provider_lines, +) from backend.util.clients import get_openai_client logger = logging.getLogger(__name__) -_MODEL = os.environ.get("BRAIN_DUMP_RECOMMEND_MODEL", "anthropic/claude-sonnet-5") +# Picking six ids off a list is a matching task, not a writing one, and +# the onboarding loading screen holds the user until it answers — so this +# runs on the fast model rather than the one that writes the greeting. +_MODEL = os.environ.get("BRAIN_DUMP_RECOMMEND_MODEL", "anthropic/claude-haiku-4-5") _TIMEOUT_SECONDS = 30 MAX_RECOMMENDATIONS = 6 @@ -66,13 +69,10 @@ async def generate_recommendations(transcript: str) -> list[RecommendedProvider] logger.warning("Brain dump recommendations: no LLM client configured") return [] - known = _known_providers() + known = known_providers() prompt = _PROMPT.format( max_recommendations=MAX_RECOMMENDATIONS, - providers="\n".join( - f"- {name}: {description}" if description else f"- {name}" - for name, description in known.items() - ), + providers=provider_lines(known), transcript=text, ) try: @@ -93,21 +93,6 @@ async def generate_recommendations(transcript: str) -> list[RecommendedProvider] return _parse_recommendations(data, set(known)) -def _known_providers() -> dict[str, str | None]: - """The live provider registry as ``{id: description}``. - - Mirrors the ``/providers`` endpoint: block modules must be imported - before AutoRegistry knows about SDK-registered providers. - """ - try: - from backend.blocks import load_all_blocks - - load_all_blocks() - except Exception as e: # static providers still work - logger.warning("Brain dump recommendations: block load failed: %s", e) - return {name: get_provider_description(name) for name in get_all_provider_names()} - - def _parse_recommendations(data: object, known: set[str]) -> list[RecommendedProvider]: items = data.get("providers") if isinstance(data, dict) else None if not isinstance(items, list): diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/service.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/service.py index 0f63d8400d3b..1ca0b62e8299 100644 --- a/autogpt_platform/backend/backend/api/features/onboarding_dump/service.py +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/service.py @@ -12,6 +12,7 @@ from fastapi import BackgroundTasks from prisma import Json from prisma.enums import BrainDumpInputMode, BrainDumpStatus +from prisma.models import OnboardingBrainDump from backend.api.features.onboarding_dump import ( db, @@ -29,6 +30,7 @@ RecommendedProvidersResponse, SuggestedPrompt, ) +from backend.copilot.db import user_has_any_session from backend.data.onboarding import format_brain_dump_for_extraction from backend.data.tally import extract_business_understanding from backend.data.understanding import ( @@ -430,7 +432,15 @@ async def _extract_and_complete( ): return _superseded_response(user_id, input_mode) - extracted = await _extract_understanding(user_id, transcript) + # The greeting reads the transcript, not the extraction, so the two + # run together rather than the loading screen paying for both in + # series. Each returns its own fallback instead of raising (see their + # docstrings), which is what keeps a gather from losing one result to + # the other's failure — the tests below hold both to that. + extracted, (greeting, suggested_prompts) = await asyncio.gather( + _extract_understanding(user_id, transcript), + intro.generate_intro(transcript), + ) extracted.additional_notes = _append_note( extracted.additional_notes, transcript, input_mode ) @@ -441,10 +451,6 @@ async def _extract_and_complete( return _superseded_response(user_id, input_mode) await upsert_business_understanding(user_id, extracted) - # Generated here, while the onboarding loading screen is still up, so - # the copilot home can render its greeting without waiting. - greeting, suggested_prompts = await intro.generate_intro(transcript) - await db.update_dump( user_id, recording_id, @@ -459,18 +465,23 @@ async def _extract_and_complete( async def _extract_understanding( user_id: str, transcript: str ) -> BusinessUnderstandingInput: - understanding = await get_business_understanding(user_id) - formatted = format_brain_dump_for_extraction( - user_name=(understanding.user_name if understanding else None) or "", - user_role=(understanding.user_role if understanding else None) or "", - transcript=transcript, - ) + """The structured understanding for ``transcript``, or an empty one. + + Never raises. A failed extraction must not cost the user their + transcript — the raw text still lands in the understanding, so the + copilot's is personalised even without structured + fields — and it runs gathered with the greeting, which a raise here + would take down with it. + """ try: + understanding = await get_business_understanding(user_id) + formatted = format_brain_dump_for_extraction( + user_name=(understanding.user_name if understanding else None) or "", + user_role=(understanding.user_role if understanding else None) or "", + transcript=transcript, + ) return await extract_business_understanding(formatted) except Exception as e: - # A failed extraction must not cost the user their transcript: the - # raw text still lands in the understanding, so the copilot's - # is personalised even without structured fields. logger.warning("Brain dump extraction failed for user %s: %s", user_id, e) return BusinessUnderstandingInput.model_construct() @@ -540,16 +551,19 @@ async def get_intro_card(user_id: str) -> IntroCardResponse: # greeting must never reappear once the first message is sent. return IntroCardResponse(path="A", greeting="", greeting_done=True) - if ( - dump is None - or dump.inputMode == BrainDumpInputMode.skipped - or not (dump.transcript or "").strip() - # A quality-rejected dump keeps its transcript on the row for - # recovery, but that text is by definition not worth reflecting - # back — falling through to Path A here would greet the user with - # a fallback built from the very content the gate refused. - or dump.errorCode in quality.QUALITY_ERROR_CODES - ): + if dump is not None and _greeting_still_writing(dump): + # The background half of the pipeline is still writing the + # greeting. The client holds its loader and polls at 1.5s — + # serving the generic fallback to a brand-new user would waste + # the personalised one that is seconds away. Answered before the + # session lookup below so that poll does not re-run it every + # cycle: someone whose dump is mid-pipeline just recorded it. + return IntroCardResponse(path="A", greeting="", greeting_pending=True) + + if await _retire_greeting_if_chatted(user_id): + return IntroCardResponse(path="A", greeting="", greeting_done=True) + + if dump is None or _nothing_to_reflect(dump): return IntroCardResponse( path="B", greeting=prompts.PATH_B_GREETING, @@ -557,15 +571,6 @@ async def get_intro_card(user_id: str) -> IntroCardResponse: ) greeting = (dump.greeting or "").strip() - if not greeting and dump.status not in ( - BrainDumpStatus.completed, - BrainDumpStatus.failed, - ): - # The background half of the pipeline is still writing the - # greeting. An empty Path A response tells the client to keep - # polling — serving the generic fallback to a brand-new user - # would waste the personalised one that is seconds away. - return IntroCardResponse(path="A", greeting="") if not greeting: # Completed before the greeting column existed, or generation # terminally failed after the transcript landed. @@ -578,6 +583,62 @@ async def get_intro_card(user_id: str) -> IntroCardResponse: ) +def _nothing_to_reflect(dump: OnboardingBrainDump) -> bool: + """Whether ``dump`` holds anything worth greeting the user about.""" + return ( + dump.inputMode == BrainDumpInputMode.skipped + or not (dump.transcript or "").strip() + # A quality-rejected dump keeps its transcript on the row for + # recovery, but that text is by definition not worth reflecting + # back — treating it as Path A would greet the user with a + # fallback built from the very content the gate refused. + or dump.errorCode in quality.QUALITY_ERROR_CODES + ) + + +def _greeting_still_writing(dump: OnboardingBrainDump) -> bool: + """Whether a Path A greeting for ``dump`` is on its way. + + A terminal status means no more is coming, whatever the column holds. + """ + if _nothing_to_reflect(dump): + return False + return not (dump.greeting or "").strip() and dump.status not in ( + BrainDumpStatus.completed, + BrainDumpStatus.failed, + ) + + +async def _retire_greeting_if_chatted(user_id: str) -> bool: + """Whether this user already has a chat session, greeting retired if so. + + ``greetingSeen`` only covers users whose first message went through + the copilot home while the flag was on — an existing account, a + session started from an expert page or a shared link, or a first send + whose completion call failed all leave it false, and the greeting + would then greet a user mid-conversation as brand new. One session is + enough to say they are past this, which is why this asks for presence + rather than a count. + + A failure here must not cost a genuinely new user their greeting, so + it answers "no" and the rest of the checks decide. + """ + try: + if not await user_has_any_session(user_id): + return False + except Exception as e: + logger.warning("Brain dump greeting: session lookup failed: %s", e) + return False + try: + # Bookkeeping behind a verdict that already stands: a failed write + # costs another count on the next load, not the answer — and must + # not 500 the card it was about to retire. + await db.mark_greeting_seen(user_id) + except Exception as e: + logger.warning("Brain dump greeting: seen flag not recorded: %s", e) + return True + + def _stored_prompts(raw: object) -> list[SuggestedPrompt]: """Rehydrate the stored prompt list, dropping anything malformed. diff --git a/autogpt_platform/backend/backend/api/features/onboarding_dump/service_test.py b/autogpt_platform/backend/backend/api/features/onboarding_dump/service_test.py index 858030cf2951..d250fac560a5 100644 --- a/autogpt_platform/backend/backend/api/features/onboarding_dump/service_test.py +++ b/autogpt_platform/backend/backend/api/features/onboarding_dump/service_test.py @@ -48,6 +48,7 @@ def __init__(self) -> None: self.row: OnboardingBrainDump | None = None self.statuses: list[BrainDumpStatus] = [] self.transcripts: list[str | None] = [] + self.greeting_seen_writes = 0 async def get_dump(self, user_id: str) -> OnboardingBrainDump | None: return self.row @@ -141,6 +142,11 @@ async def mark_failed( errorCode=error_code, ) + async def mark_greeting_seen(self, user_id: str) -> None: + if self.row is not None: + self.row.greetingSeen = True + self.greeting_seen_writes += 1 + @pytest.fixture(autouse=True) def dumps(mocker: MockerFixture) -> DumpStore: @@ -152,6 +158,7 @@ def dumps(mocker: MockerFixture) -> DumpStore: mocker.patch(f"{module}.update_dump", new=store.update_dump) mocker.patch(f"{module}.mark_failed", new=store.mark_failed) mocker.patch(f"{module}.claim_transition", new=store.claim_transition) + mocker.patch(f"{module}.mark_greeting_seen", new=store.mark_greeting_seen) return store @@ -814,6 +821,119 @@ async def test_intro_card_takes_path_a_with_the_stored_greeting(dumps: DumpStore assert card.transcript == TRANSCRIPT +@pytest.mark.asyncio +async def test_intro_card_is_withheld_from_a_user_who_already_has_a_session( + dumps: DumpStore, has_session: AsyncMock +): + has_session.return_value = True + await dumps.start_dump(USER_ID, RECORDING_ID, BrainDumpInputMode.voice) + await dumps.update_dump( + USER_ID, + RECORDING_ID, + status=BrainDumpStatus.completed, + transcript=TRANSCRIPT, + greeting="You mentioned the weekly order emails.", + ) + + card = await service.get_intro_card(USER_ID) + + assert card.greeting_done is True + assert card.greeting == "" + # Recorded server-side so the count is only paid once. + assert dumps.greeting_seen_writes == 1 + + +@pytest.mark.asyncio +async def test_intro_card_is_withheld_from_a_chatting_user_without_a_dump_row( + dumps: DumpStore, has_session: AsyncMock +): + has_session.return_value = True + + card = await service.get_intro_card(USER_ID) + + assert card.greeting_done is True + assert card.greeting == "" + assert dumps.greeting_seen_writes == 1 + + +@pytest.mark.asyncio +async def test_intro_card_is_still_withheld_when_the_seen_flag_cannot_be_written( + has_session: AsyncMock, mocker: MockerFixture +): + # The session lookup is the verdict; the flag only saves paying for it + # again. A failed write must not 500 the card it was retiring. + has_session.return_value = True + mocker.patch( + "backend.api.features.onboarding_dump.db.mark_greeting_seen", + new=AsyncMock(side_effect=RuntimeError("database down")), + ) + + card = await service.get_intro_card(USER_ID) + + assert card.greeting_done is True + assert card.greeting == "" + + +@pytest.mark.asyncio +async def test_intro_card_still_greets_when_the_session_lookup_fails( + dumps: DumpStore, has_session: AsyncMock +): + has_session.side_effect = RuntimeError("database down") + await dumps.start_dump(USER_ID, RECORDING_ID, BrainDumpInputMode.voice) + await dumps.update_dump( + USER_ID, + RECORDING_ID, + status=BrainDumpStatus.completed, + transcript=TRANSCRIPT, + greeting="You mentioned the weekly order emails.", + ) + + card = await service.get_intro_card(USER_ID) + + assert card.greeting_done is False + assert card.greeting == "You mentioned the weekly order emails." + + +@pytest.mark.asyncio +async def test_intro_card_reports_pending_while_the_greeting_is_generating( + dumps: DumpStore, +): + await dumps.start_dump(USER_ID, RECORDING_ID, BrainDumpInputMode.voice) + await dumps.update_dump( + USER_ID, + RECORDING_ID, + status=BrainDumpStatus.extracting, + transcript=TRANSCRIPT, + ) + + card = await service.get_intro_card(USER_ID) + + assert card.greeting_pending is True + assert card.greeting == "" + assert card.greeting_done is False + + +@pytest.mark.asyncio +async def test_intro_card_skips_the_session_lookup_while_the_greeting_is_pending( + dumps: DumpStore, has_session: AsyncMock +): + # The client polls this endpoint every 1.5s while pending, and a + # mid-pipeline dump belongs to someone who just recorded it — asking + # the sessions table on every cycle would buy nothing. + await dumps.start_dump(USER_ID, RECORDING_ID, BrainDumpInputMode.voice) + await dumps.update_dump( + USER_ID, + RECORDING_ID, + status=BrainDumpStatus.extracting, + transcript=TRANSCRIPT, + ) + + card = await service.get_intro_card(USER_ID) + + assert card.greeting_pending is True + has_session.assert_not_awaited() + + @pytest.mark.asyncio async def test_intro_card_takes_path_b_when_the_user_skipped(dumps: DumpStore): await dumps.start_dump(USER_ID, RECORDING_ID, BrainDumpInputMode.skipped) @@ -1021,3 +1141,66 @@ async def test_intro_generation_degrades_instead_of_raising(mocker: MockerFixtur assert greeting == intro.fallback_intro(TRANSCRIPT)[0] assert suggested == intro.fallback_prompts() + + +@pytest.mark.asyncio +async def test_a_failed_extraction_does_not_cost_the_greeting( + dumps: DumpStore, extraction: dict[str, AsyncMock], mocker: MockerFixture +): + # Extraction and the greeting run gathered, so a raise on either side + # would abandon the other's result. Each is supposed to answer with a + # fallback instead — this holds the extraction half to that. + extraction["extract_business_understanding"].side_effect = RuntimeError("llm down") + mocker.patch( + "backend.api.features.onboarding_dump.intro.generate_intro", + new=AsyncMock(return_value=("You mentioned the order emails.", [])), + ) + await start_voice_take(dumps) + + await finalize_voice() + + assert dumps.row is not None + assert dumps.row.status == BrainDumpStatus.completed + assert dumps.row.greeting == "You mentioned the order emails." + # The transcript still reaches the understanding without the fields. + extraction["upsert_business_understanding"].assert_awaited_once() + + +@pytest.mark.asyncio +async def test_an_unreadable_understanding_does_not_cost_the_greeting( + dumps: DumpStore, extraction: dict[str, AsyncMock], mocker: MockerFixture +): + # The same contract one call earlier: reading the existing + # understanding is a database round trip that can fail on its own. + extraction["get_business_understanding"].side_effect = RuntimeError("database down") + mocker.patch( + "backend.api.features.onboarding_dump.intro.generate_intro", + new=AsyncMock(return_value=("You mentioned the order emails.", [])), + ) + await start_voice_take(dumps) + + await finalize_voice() + + assert dumps.row is not None + assert dumps.row.status == BrainDumpStatus.completed + assert dumps.row.greeting == "You mentioned the order emails." + + +@pytest.mark.asyncio +async def test_a_failed_greeting_does_not_cost_the_extraction( + dumps: DumpStore, extraction: dict[str, AsyncMock], mocker: MockerFixture +): + # The greeting half of the same contract: a broken LLM call resolves + # to the template, and the extraction alongside it still lands. + mocker.patch( + "backend.api.features.onboarding_dump.intro.get_openai_client", + return_value=None, + ) + await start_voice_take(dumps) + + await finalize_voice() + + assert dumps.row is not None + assert dumps.row.status == BrainDumpStatus.completed + assert dumps.row.greeting == intro.fallback_intro(TRANSCRIPT)[0] + extraction["upsert_business_understanding"].assert_awaited_once() diff --git a/autogpt_platform/backend/backend/copilot/db.py b/autogpt_platform/backend/backend/copilot/db.py index 34863affb4eb..c936fbc44583 100644 --- a/autogpt_platform/backend/backend/copilot/db.py +++ b/autogpt_platform/backend/backend/copilot/db.py @@ -687,6 +687,22 @@ async def get_user_session_count( return rows[0]["count"] if rows else 0 +async def user_has_any_session(user_id: str) -> bool: + """Whether the user has at least one visible chat session. + + The presence-only counterpart to :func:`get_user_session_count`, for + callers that only compare the total against zero: it stops at the + first matching row instead of scanning every session the user owns. + """ + rows = await db.query_raw_with_schema( + 'SELECT 1 FROM {schema_prefix}"ChatSession" WHERE "userId" = $1 AND ' + + _EXCLUDE_DREAM_SESSIONS_SQL + + " LIMIT 1", + user_id, + ) + return bool(rows) + + def _escape_like(value: str) -> str: """Escape LIKE wildcards so ``title_contains`` matches literally. diff --git a/autogpt_platform/backend/backend/copilot/db_session_listing_test.py b/autogpt_platform/backend/backend/copilot/db_session_listing_test.py index a029164260f1..f50f281d76c8 100644 --- a/autogpt_platform/backend/backend/copilot/db_session_listing_test.py +++ b/autogpt_platform/backend/backend/copilot/db_session_listing_test.py @@ -28,6 +28,7 @@ get_user_chat_sessions, get_user_session_count, update_chat_session_title, + user_has_any_session, ) from backend.copilot.model import ChatSessionMetadata from backend.util.json import SafeJson @@ -132,6 +133,32 @@ async def test_count_returns_zero_when_query_yields_no_rows(): assert await get_user_session_count("u1") == 0 +@pytest.mark.asyncio +async def test_presence_query_stops_at_the_first_row_and_keeps_the_dream_filter(): + """The greeting gate reads presence, so it must not scan every session. + + A regression in either half — the dream exclusion or the ``LIMIT 1`` — + is silent at the call site: ``_retire_greeting_if_chatted`` swallows + failures and answers "no", so every new user would keep a greeting + they have already outgrown. + """ + raw = AsyncMock(return_value=[{"?column?": 1}]) + with patch(_RAW_QUERY_TARGET, raw): + assert await user_has_any_session("u1") is True + + query = raw.call_args.args[0] + assert _NULL_SAFE_DREAM_FILTER in query + assert "LIMIT 1" in query + assert raw.call_args.args[1:] == ("u1",) + + +@pytest.mark.asyncio +async def test_presence_is_false_when_the_user_owns_no_visible_session(): + raw = AsyncMock(return_value=[]) + with patch(_RAW_QUERY_TARGET, raw): + assert await user_has_any_session("u1") is False + + # ---------- NULL semantics against the real database ---------- diff --git a/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/usePreparingStep.test.ts b/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/usePreparingStep.test.ts index b247be35db08..717c86b70607 100644 --- a/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/usePreparingStep.test.ts +++ b/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/usePreparingStep.test.ts @@ -10,7 +10,7 @@ import { usePreparingStep } from "../usePreparingStep"; const START_DELAY_MS = 300; const GENERIC_TOTAL_MS = 4_000; const DUMP_TOTAL_MS = 10_000; -const RECOMMENDATIONS_MAX_WAIT_MS = 60_000; +const RECOMMENDATIONS_MAX_WAIT_MS = 15_000; const GENERIC_CHECKLIST = [ "Personalizing your experience", @@ -214,7 +214,9 @@ describe("usePreparingStep — recommendation gate (dump path)", () => { }); await advance(START_DELAY_MS); - await advance(DUMP_TOTAL_MS + 5_000); + // Short of the ceiling, so this asserts the gate rather than the + // give-up timer that eventually overrides it. + await advance(DUMP_TOTAL_MS + 2_000); expect(onComplete).not.toHaveBeenCalled(); expect(result.current.completedItems).toBe(DUMP_CHECKLIST.length - 1); @@ -231,15 +233,16 @@ describe("usePreparingStep — recommendation gate (dump path)", () => { }); await advance(START_DELAY_MS); - await advance(DUMP_TOTAL_MS + 2_000); + await advance(DUMP_TOTAL_MS + 500); expect(onComplete).not.toHaveBeenCalled(); mockRecommendations(true); // Next poll (2.5s cadence) picks up ready=true; the response settles // at the end of the first window, so a second window lets the ticker - // observe it and complete the run. + // observe it and complete the run. Kept inside the give-up ceiling so + // the poll is what releases the gate, not the timer. await advance(3_000); - await advance(1_000); + await advance(500); expect(onComplete).toHaveBeenCalledTimes(1); expect(result.current.progress).toBe(100); diff --git a/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/usePreparingStep.ts b/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/usePreparingStep.ts index ae423c15bd68..f8984b65b5a9 100644 --- a/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/usePreparingStep.ts +++ b/autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/usePreparingStep.ts @@ -27,7 +27,10 @@ const BRAIN_DUMP_DURATION_MS = 10_000; const RECOMMENDATIONS_POLL_MS = 2_500; // A job the backend never finished (process restart mid-run) must not // strand the user on this screen — advance anyway after this ceiling. -const RECOMMENDATIONS_MAX_WAIT_MS = 60_000; +// The recommender runs on the fast model, so a job still unanswered this +// late is one that is not coming; the connect dialog falls back to popular +// providers rather than making the user watch a bar for another minute. +const RECOMMENDATIONS_MAX_WAIT_MS = 15_000; // While waiting on the job the bar parks just short of full so it still // reads as "almost there", not "stuck at done". const WAITING_PROGRESS_CAP = 95; diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsx index 6e7476a86190..76c3972c3c32 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsx @@ -22,6 +22,7 @@ import { usePulseChips } from "../PulseChips/usePulseChips"; import { Flag, useGetFlag } from "@/services/feature-flags/use-get-flag"; import type { WorkspaceAttachment } from "../../helpers/workspaceAttachments"; import { EmptyHero } from "./components/EmptyHero"; +import { GreetingLoader } from "./components/GreetingLoader"; import { CopilotHome } from "../CopilotHome/CopilotHome"; import { RecipientChip } from "../ChatInput/components/RecipientChip"; import { useRecipientPicker } from "./useRecipientPicker"; @@ -130,17 +131,14 @@ export function EmptySession({ onSelectPrompt={onSend} disabled={isComposerDisabled} /> + ) : intro.isAwaitingGreeting ? ( + // Behind the welcome modal's blur and for as long as the + // pipeline is still writing. The orb it renders is the same + // element the card above puts in its heading, so the swap + // moves it there rather than replacing it. + ) : ( - // The regular hero also renders behind the welcome modal's - // blur and while the greeting is still generating — it swaps - // to the greeting the moment the real one arrives. Through - // that whole flow it wears the greeting page's own layout so - // the heading never moves when the swap happens. - + )} {!intro.isVisible && diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/__tests__/greeting-loader.test.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/__tests__/greeting-loader.test.tsx new file mode 100644 index 000000000000..4eb615e8194f --- /dev/null +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/__tests__/greeting-loader.test.tsx @@ -0,0 +1,17 @@ +import { render, screen } from "@/tests/integrations/test-utils"; +import { describe, expect, it } from "vitest"; + +import { GreetingLoader } from "../components/GreetingLoader"; + +describe("GreetingLoader", () => { + it("announces the wait to assistive tech", () => { + // The orb is decorative and the composer is withheld while this + // renders, so the status region and its label are the only signal a + // screen reader user gets that something is coming. + render(); + + const status = screen.getByRole("status"); + expect(status).toBeDefined(); + expect(status.textContent).toContain("Writing your greeting"); + }); +}); diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/EmptyHero.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/EmptyHero.tsx index 85acaa720a94..eb69590d6a45 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/EmptyHero.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/EmptyHero.tsx @@ -1,71 +1,30 @@ "use client"; import { Text } from "@/components/atoms/Text/Text"; -import { GlassOrb } from "@/components/molecules/GlassOrb/GlassOrb"; import { TextGenerateEffect } from "@/components/ui/text-generate-effect"; -import { cn } from "@/lib/utils"; -import { - ORB_PURPLE, - SMALL_ORB_PARAMS, -} from "../../OnboardingIntroCard/OnboardingIntroCard"; import { EditNameDialog } from "./EditNameDialog/EditNameDialog"; interface Props { name: string; - isAwaitingGreeting: boolean; - isGreetingFlow: boolean; } -// The regular empty-session hero. It also stands in for the greeting page -// while the greeting is still generating, and in that mode the heading row -// is laid out exactly where OnboardingIntroCard is about to put it — same -// left edge, same type — so the swap leaves it where it already was -// instead of throwing it in from the centre of the page. -export function EmptyHero({ name, isAwaitingGreeting, isGreetingFlow }: Props) { +// The regular empty-session hero. While a greeting is being written +// GreetingLoader renders in its place instead, and its orb travels into +// the intro card's heading under a shared layout id. +export function EmptyHero({ name }: Props) { return ( <> -
- {isAwaitingGreeting && ( - - - - )} - - Hey,{" "} - {isGreetingFlow ? ( - {name} - ) : ( - {name} - )} - {/* The greeting page carries no name editing, so keeping the - trigger here would pop it out mid-swap. */} - {!isGreetingFlow && } +
+ + Hey, {name} +
- {/* Held back with the composer: while the greeting decision is - pending this line appearing then vanishing read as the page - changing its mind on refresh. */} - {!isAwaitingGreeting && ( - - )} + ); } diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx new file mode 100644 index 000000000000..75b4673406d1 --- /dev/null +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/GreetingLoader.tsx @@ -0,0 +1,63 @@ +"use client"; + +import { GlassOrb } from "@/components/molecules/GlassOrb/GlassOrb"; +import { motion, useReducedMotion } from "framer-motion"; +import { + GREETING_ORB_LAYOUT_ID, + ORB_FLIP_TRANSITION, + ORB_FLIP_TRANSITION_REDUCED, + SMALL_ORB_PARAMS, +} from "../../../helpers/greetingOrb"; + +// What the page is while the greeting is still being written: the orb, +// alone, breathing in the middle of the empty session. It carries the +// intro card's `layoutId`, so when the greeting lands this exact element +// travels into the heading instead of being swapped out for a copy. +export function GreetingLoader() { + const prefersReducedMotion = useReducedMotion(); + + return ( +
+ {/* The orb is decorative and hidden from assistive tech, so the wait + needs saying out loud — the composer is withheld until it ends. */} + Writing your greeting + + + + + +
+ ); +} diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/OnboardingIntroCard.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/OnboardingIntroCard.tsx index 98a949bd5815..4974d8f4ec89 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/OnboardingIntroCard.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/OnboardingIntroCard.tsx @@ -1,8 +1,14 @@ "use client"; import { GlassOrb } from "@/components/molecules/GlassOrb/GlassOrb"; -import type { GlassParams } from "@/components/molecules/GlassOrb/GlassSurface"; import type { SuggestedPrompt } from "@/app/api/__generated__/models/suggestedPrompt"; +import { + GREETING_ORB_LAYOUT_ID, + ORB_FLIP_TRANSITION, + ORB_FLIP_TRANSITION_REDUCED, + ORB_PURPLE, + SMALL_ORB_PARAMS, +} from "../../helpers/greetingOrb"; import { Icon } from "@/components/atoms/Icon/Icon"; import { Text } from "@/components/atoms/Text/Text"; import { useToast } from "@/components/molecules/Toast/use-toast"; @@ -75,27 +81,11 @@ interface Props { disabled?: boolean; } -// The default glass params are tuned for the big onboarding orb; at 32px -// that much frost and distortion collapses into a flat purple ball. Light -// frost + gentle refraction keeps the drifting blobs readable this small. -// Also rendered by EmptySession's hero while the greeting is on its way, -// so the orb is already on screen before the reveal. -export const SMALL_ORB_PARAMS: GlassParams = { - frost: 1.5, - saturation: 1.5, - tint: 0.12, - edge: 0.55, - distortion: 8, - ringWidth: 1, - ringDepth: 2, - ringDark: 0.25, -}; - -// The purple the orb's blobs blend into — the name mirrors it. Shared with -// the hero heading this card replaces so the swap is invisible. -export const ORB_PURPLE = "#8a4dff"; - -const GREETING_START = 0.35; +// The orb travels into this card's heading from the loader, so the +// heading is revealed on its arrival and everything below waits for the +// trip to finish. +const HEADING_START = 0.2; +const GREETING_START = 0.5; const WORD_STAGGER = 0.08; const ROW_STAGGER = 0.12; const ROW_START_BUFFER = 0.3; @@ -169,40 +159,51 @@ export function OnboardingIntroCard({ className="mb-8 w-full max-w-[48rem] text-left" data-testid="onboarding-intro-card" > - {/* Not revealed: this exact row is already on screen as the hero's - heading while the greeting generates, in this exact spot. Fading - and rising it here would blink a heading that never moved. */}
- + {/* Not revealed — it flies in from the loader's centre under its + own layout animation. Fading it too would fight that trip. */} + - - - Hey, {name} - + + + + Hey, {name} + + {transcript && ( - - - - - - {isCopied ? "Copied!" : "Copy everything you told me"} - - + + + + + + + {isCopied ? "Copied!" : "Copy everything you told me"} + + + )}
diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/OnboardingIntroCard.test.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/OnboardingIntroCard.test.tsx index e4b542fa3522..d2deb49cd313 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/OnboardingIntroCard.test.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/OnboardingIntroCard.test.tsx @@ -243,7 +243,7 @@ describe("introRevealTimings", () => { const short = introRevealTimings("Hello there", 0); const long = introRevealTimings("Hello there, this is a longer line", 0); - expect(short.promptsStart).toBeCloseTo(0.35 + 2 * 0.08 + 0.3, 5); + expect(short.promptsStart).toBeCloseTo(0.5 + 2 * 0.08 + 0.3, 5); expect(long.promptsStart).toBeGreaterThan(short.promptsStart); expect(long.footerStart).toBeGreaterThan(short.footerStart); expect(long.composerStart).toBeGreaterThan(short.composerStart); @@ -271,7 +271,7 @@ describe("introRevealTimings", () => { 0, ); - expect(promptsStart).toBeCloseTo(0.65, 5); + expect(promptsStart).toBeCloseTo(0.8, 5); expect(footerStart).toBeGreaterThan(promptsStart); expect(composerStart).toBeGreaterThan(footerStart); }); diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/useOnboardingIntroCard.test.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/useOnboardingIntroCard.test.tsx index b214873e80f5..b6caf22cfba9 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/useOnboardingIntroCard.test.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/__tests__/useOnboardingIntroCard.test.tsx @@ -52,6 +52,7 @@ const PENDING_INTRO: IntroCardResponse = { path: "A", greeting: "", greeting_done: false, + greeting_pending: true, prompts: [], }; @@ -342,6 +343,28 @@ describe("useOnboardingIntroCard — the greeting itself", () => { expect(result.current.isVisible).toBe(false); expect(result.current.greeting).toBe(""); + // The orb sits centered for exactly this window; only the greeting + // itself anchors the page to the top. + expect(result.current.anchorTop).toBe(false); + }); + + it("releases the loader when the server sends no greeting and no pending flag", async () => { + // A terminally-empty Path A (generation gave up server-side): there is + // nothing coming, so holding the orb up would strand the composer. + server.use( + getGetBrainDumpIntroMockHandler200({ + path: "A", + greeting: "", + greeting_done: false, + greeting_pending: false, + prompts: [], + }), + ); + + const { result } = renderIntro(); + await waitFor(() => expect(result.current.isAwaitingGreeting).toBe(false)); + + expect(result.current.isVisible).toBe(false); }); it("polls past the pending answer and reveals the greeting when it lands", async () => { @@ -380,6 +403,32 @@ describe("useOnboardingIntroCard — the greeting itself", () => { expect(urls).toHaveLength(1); }); + it("never paints the card for a done verdict that still carries a greeting", async () => { + // The verdict is cached in an effect, which runs after the render that + // first saw it — reading only the cached copy flashed the card for the + // paint in between. + countIntroRequests({ + path: "A", + greeting: "Welcome back.", + greeting_done: true, + }); + const painted: boolean[] = []; + + renderHook( + () => { + const value = useOnboardingIntroCard(); + painted.push(value.isVisible); + return value; + }, + { wrapper: makeWrapper() }, + ); + await waitFor(() => + expect(window.localStorage.getItem(GREETING_DONE_KEY)).toBe("user-1"), + ); + + expect(painted).not.toContain(true); + }); + it("writes no cache entry while the user record is still loading", async () => { authUser.current = null; countIntroRequests({ diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/useOnboardingIntroCard.ts b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/useOnboardingIntroCard.ts index aa70b3af0f9a..67b295aa7514 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/useOnboardingIntroCard.ts +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingIntroCard/useOnboardingIntroCard.ts @@ -18,7 +18,7 @@ import { import { useEffect, useState } from "react"; // While the pipeline is still writing the greeting the intro endpoint -// answers with an empty Path A — poll at this cadence until the real +// answers `greeting_pending` — poll at this cadence until the real // greeting lands (or the pipeline terminally resolves). const PENDING_POLL_MS = 1500; // A pipeline killed between transcription and completion leaves the dump @@ -111,8 +111,7 @@ export function useOnboardingIntroCard() { const latest = query.state.data; if (!latest || latest.status !== 200) return false; const body = latest.data; - // Empty Path A greeting = pipeline still generating; keep asking. - if (!body.greeting_done && body.path === "A" && !body.greeting) { + if (!body.greeting_done && body.greeting_pending) { return PENDING_POLL_MS; } return false; @@ -134,7 +133,7 @@ export function useOnboardingIntroCard() { (isFlagReady && !isBrainDumpEnabled) || data !== undefined || isError; const serverSaysDone = Boolean(intro?.greeting_done); const isPendingPerServer = Boolean( - intro && !intro.greeting_done && intro.path === "A" && !intro.greeting, + intro && !intro.greeting_done && intro.greeting_pending, ); const isPendingGeneration = isPendingPerServer && !gaveUpWaiting; @@ -160,42 +159,43 @@ export function useOnboardingIntroCard() { setIsWelcomeOpen(false); } - // The whole greeting flow reads top-down like a letter, so it anchors - // to the top from its first visible frame — flipping the container - // from centered to top only when the greeting arrived made the "Hey" - // heading visibly jump. `isWelcomeOpen` needs no flag check (only the - // gated handoff ever sets it) and is seeded synchronously, so the - // fresh-out-of-onboarding user is anchored before LaunchDarkly answers. - const isGreetingFlow = - isWelcomeOpen || - (Boolean(isBrainDumpEnabled) && - (!hasIntroAnswer || isPendingGeneration || Boolean(intro?.greeting))); - // Holding the composer is only ever right while this flow is in play: // the overlay is up (seeded synchronously from the handoff) or the flag // already reads on. Holding on "LaunchDarkly has not answered" alone // hid the composer on every flag-off /copilot load until it did. const isGreetingExpected = isWelcomeOpen || Boolean(isBrainDumpEnabled); + // `serverSaysDone` as well as `isDone`: the effect that caches the verdict + // runs after render, so a payload that still carries a greeting would + // otherwise flash the card for one paint before it takes effect. + const isVisible = + isMounted && + !isDone && + !serverSaysDone && + !isWelcomeOpen && + hasIntroAnswer && + Boolean(intro?.greeting); return { - // The page renders the regular hero behind the welcome modal and - // while the greeting is generating; it swaps to the greeting the - // moment the real one arrives. - isVisible: - isMounted && - !isDone && - !isWelcomeOpen && - hasIntroAnswer && - Boolean(intro?.greeting), + isVisible, // The greeting is on its way (modal up, no server answer yet, or the - // pipeline still writing) — the hero shows the orb and holds the - // composer back until the greeting page takes over. + // pipeline still writing): the page shows the orb alone, centered, and + // holds the composer until the greeting takes over. The orb then + // travels into the heading rather than being replaced by it, so this + // must go false in the same commit `isVisible` goes true. + // + // The pre-mount frame counts: the server render cannot know any of + // this, and letting the composer through on that one frame flashed it + // before the greeting page arrived. The loader fades in on a delay, so + // a flag-off user never sees the orb it renders for that frame. isAwaitingGreeting: !isMounted || (!isDone && isGreetingExpected && (isWelcomeOpen || !hasIntroAnswer || isPendingGeneration)), - anchorTop: isMounted && !isDone && isGreetingFlow, + // Only the greeting itself reads top-down like a letter. The loader + // stays centered — that is the distance the orb travels when the + // greeting lands. + anchorTop: isVisible, isWelcomeOpen: !isDone && isWelcomeOpen, closeWelcome, greeting: intro?.greeting ?? "", diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/ConnectToolsPanel.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/ConnectToolsPanel.tsx index 988464037094..d2ea98c4e604 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/ConnectToolsPanel.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/ConnectToolsPanel.tsx @@ -40,6 +40,7 @@ export function ConnectToolsPanel({ onBack, onNext }: Props) { setQuery, providers, recommendedProviders, + isPersonalized, isLoading, isError, error, @@ -175,7 +176,9 @@ export function ConnectToolsPanel({ onBack, onNext }: Props) { ) : recommendedProviders.length > 0 ? (
- Recommended from our conversation + {isPersonalized + ? "Recommended from our conversation" + : "Popular places to start"}
    {recommendedProviders.map((provider) => ( diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers-flag-off.test.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers-flag-off.test.tsx new file mode 100644 index 000000000000..3c45645164c6 --- /dev/null +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers-flag-off.test.tsx @@ -0,0 +1,57 @@ +import { server } from "@/mocks/mock-server"; +import { render, screen } from "@/tests/integrations/test-utils"; +import { http, HttpResponse } from "msw"; +import { describe, expect, it, vi } from "vitest"; + +// With the flag off the recommendation endpoint 404s and is never called, +// so nothing will ever settle the section on its own — the panel has to +// treat "flag off" as an answer or it shows an empty list forever. +vi.mock("@/services/feature-flags/use-get-flag", async (importOriginal) => { + const actual = + await importOriginal< + typeof import("@/services/feature-flags/use-get-flag") + >(); + return { ...actual, useGetFlag: () => false }; +}); + +import { ConnectToolsPanel } from "../ConnectToolsPanel"; + +const PROVIDERS_URL = + "http://localhost:3000/api/proxy/api/integrations/providers"; +const CREDENTIALS_URL = + "http://localhost:3000/api/proxy/api/integrations/credentials"; +const RECOMMENDED_URL = + "http://localhost:3000/api/proxy/api/onboarding/brain-dump/recommended-providers"; + +describe("ConnectToolsPanel — brain dump flag off", () => { + it("shows the popular providers without asking for recommendations", async () => { + let recommendationRequests = 0; + server.use( + http.get(PROVIDERS_URL, () => + HttpResponse.json([ + { + name: "notion", + description: "Docs and wikis", + supported_auth_types: ["oauth2"], + }, + { + name: "slack", + description: "Team chat", + supported_auth_types: ["oauth2"], + }, + ]), + ), + http.get(CREDENTIALS_URL, () => HttpResponse.json([])), + http.get(RECOMMENDED_URL, () => { + recommendationRequests += 1; + return HttpResponse.json({ ready: true, providers: [] }); + }), + ); + + render(); + + expect(await screen.findByText("Popular places to start")).toBeDefined(); + expect(screen.getByRole("button", { name: /Notion/ })).toBeDefined(); + expect(recommendationRequests).toBe(0); + }); +}); diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers.test.tsx b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers.test.tsx index fe3dabfe13b0..58e6c8d8b22b 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers.test.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/__tests__/recommended-providers.test.tsx @@ -27,6 +27,7 @@ const RECOMMENDED_URL = const POLL_INTERVAL_MS = 2_500; const RECOMMENDED_HEADING = "Recommended from our conversation"; +const FALLBACK_HEADING = "Popular places to start"; const SEARCH_PROMPT = "Search to find a service to connect."; type RecommendedResponse = { @@ -68,6 +69,24 @@ function scriptRecommendations(...responses: RecommendedResponse[]) { return hits; } +// A registry holding only one of the preferred fallback ids, so the rest +// of the section has to be padded out of what is left. +function stubRegistryWithoutMostPreferredIds() { + const names = ["slack", "airtable", "linear", "stripe", "hubspot", "asana"]; + server.use( + http.get(PROVIDERS_URL, () => + HttpResponse.json( + names.map((name) => ({ + name, + description: `${name} description`, + supported_auth_types: ["oauth2"], + })), + ), + ), + http.get(CREDENTIALS_URL, () => HttpResponse.json([])), + ); +} + function renderPanel() { return render(); } @@ -133,21 +152,72 @@ describe("ConnectToolsPanel — recommendations", () => { expect(screen.getAllByText(RECOMMENDED_HEADING)).toHaveLength(1); }); - it("shows the search prompt and stops polling when the model recommends nothing", async () => { + it("renders one card per provider when the model names the same one twice", async () => { + stubRegistry(); + scriptRecommendations({ + ready: true, + providers: [ + { provider: "notion", reason: "You mentioned meeting notes" }, + { provider: "notion", reason: "And your roadmap lives there" }, + ], + }); + + renderPanel(); + + expect(await screen.findByText(RECOMMENDED_HEADING)).toBeDefined(); + expect(screen.getAllByRole("button", { name: /Notion/ })).toHaveLength(1); + // The first reason wins, so the duplicate does not quietly rewrite it. + expect(screen.getByText("You mentioned meeting notes")).toBeDefined(); + expect(screen.queryByText("And your roadmap lives there")).toBeNull(); + }); + + it("falls back to popular providers and stops polling when the model recommends nothing", async () => { vi.useFakeTimers({ shouldAdvanceTime: true }); stubRegistry(); - // An empty array is a real answer, not "still working". + // An empty array is a real answer, not "still working" — but an empty + // panel reads as broken, so the generic picks take over. const hits = scriptRecommendations({ ready: true, providers: [] }); renderPanel(); - await waitFor(() => expect(screen.getByText(SEARCH_PROMPT)).toBeDefined()); + expect(await screen.findByText(FALLBACK_HEADING)).toBeDefined(); + expect(screen.getByRole("button", { name: /Notion/ })).toBeDefined(); + expect(screen.getByRole("button", { name: /Slack/ })).toBeDefined(); + // The copy must not claim these came out of the conversation. expect(screen.queryByText(RECOMMENDED_HEADING)).toBeNull(); + expect(screen.queryByText(SEARCH_PROMPT)).toBeNull(); await pollTimes(4); expect(hits).toHaveLength(1); }); + it("pads the popular fallback out of the registry when the preferred providers are missing", async () => { + // A deployment without most of the preferred ids must still fill the + // section rather than showing the one it happens to have. + stubRegistryWithoutMostPreferredIds(); + scriptRecommendations({ ready: true, providers: [] }); + + renderPanel(); + + expect(await screen.findByText(FALLBACK_HEADING)).toBeDefined(); + expect(screen.getAllByRole("listitem")).toHaveLength(6); + // The one preferred id present still leads the list. + expect(screen.getAllByRole("listitem")[0].textContent).toContain("Slack"); + }); + + it("falls back to popular providers when the recommendation request fails", async () => { + stubRegistry(); + // A rejected request never reaches `data`, so it is the query's error + // state — not a status — that has to settle the section. + server.use(http.get(RECOMMENDED_URL, () => HttpResponse.error())); + + renderPanel(); + + expect(await screen.findByText(FALLBACK_HEADING)).toBeDefined(); + expect(screen.getByRole("button", { name: /Notion/ })).toBeDefined(); + expect(screen.queryByText(RECOMMENDED_HEADING)).toBeNull(); + }); + it("keeps polling while the job is unfinished and renders the answer when it lands", async () => { vi.useFakeTimers({ shouldAdvanceTime: true }); stubRegistry(); @@ -164,7 +234,10 @@ describe("ConnectToolsPanel — recommendations", () => { renderPanel(); + // Nothing generic while the job may still answer — swapping the picks + // out from under the user is worse than a beat of empty space. await waitFor(() => expect(screen.getByText(SEARCH_PROMPT)).toBeDefined()); + expect(screen.queryByText(FALLBACK_HEADING)).toBeNull(); expect(hits).toHaveLength(1); await pollTimes(2); @@ -172,6 +245,7 @@ describe("ConnectToolsPanel — recommendations", () => { await waitFor(() => expect(screen.getByRole("button", { name: /Notion/ })).toBeDefined(), ); + expect(screen.getByText(RECOMMENDED_HEADING)).toBeDefined(); expect(screen.queryByText(SEARCH_PROMPT)).toBeNull(); expect(hits).toHaveLength(3); diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/helpers.ts b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/helpers.ts new file mode 100644 index 000000000000..6250401a4919 --- /dev/null +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/helpers.ts @@ -0,0 +1,39 @@ +import type { ConnectableProvider } from "@/components/contextual/IntegrationsPanel/components/ConnectServiceDialog/helpers"; + +// What the section shows when the model picked nothing — a thin +// transcript, a skipped dump, or a job that never landed. An empty +// "Connect your tools" panel reads as a broken dialog, and these are what +// most people wire up first anyway. +const FALLBACK_PROVIDER_IDS = [ + "google", + "slack", + "notion", + "github", + "discord", + "todoist", +]; +const FALLBACK_COUNT = 6; + +// An id the model named twice keeps its first reason, so the section never +// renders two cards under the same React key. +export function firstMentionOfEachProvider( + recommendations: T[], +) { + const named = new Set(); + return recommendations.filter((recommendation) => { + if (named.has(recommendation.provider)) return false; + named.add(recommendation.provider); + return true; + }); +} + +// The preferred ids in order, padded from the rest of the registry so a +// deployment missing some of them still fills the section. +export function fallbackProviders(all: ConnectableProvider[]) { + const byPreference = FALLBACK_PROVIDER_IDS.map((id) => + all.find((provider) => provider.id === id), + ).filter((provider) => provider !== undefined); + const picked = new Set(byPreference.map((provider) => provider.id)); + const padding = all.filter((provider) => !picked.has(provider.id)); + return [...byPreference, ...padding].slice(0, FALLBACK_COUNT); +} diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts index 226e025efbf9..8c391c3a1b2b 100644 --- a/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/components/OnboardingWelcomeDialog/useConnectToolsPanel.ts @@ -17,6 +17,7 @@ import { import { useDebouncedValue } from "@/hooks/useDebouncedValue"; import { Flag, useGetFlag } from "@/services/feature-flags/use-get-flag"; import { useState } from "react"; +import { fallbackProviders, firstMentionOfEachProvider } from "./helpers"; const POLL_INTERVAL_MS = 2_500; // ~1 minute of polling; a job the backend never finished (process restart @@ -79,8 +80,10 @@ export function useConnectToolsPanel() { : []; // The model's reason replaces the generic provider description — that // line is what makes the section feel picked for this user. Unknown ids - // (provider renamed or removed since the job ran) are dropped. - const recommendedProviders = recommendations + // (provider renamed or removed since the job ran) are dropped, and an id + // the model named twice keeps its first reason rather than rendering two + // cards under the same React key. + const personalizedProviders = firstMentionOfEachProvider(recommendations) .map((recommendation): ConnectableProvider | null => { const provider = allProviders.find( (p) => p.id === recommendation.provider, @@ -93,6 +96,23 @@ export function useConnectToolsPanel() { }) .filter((provider) => provider !== null); + // Only fall back once the job has actually answered — `ready`, a status + // it will not recover from, or the flag being off. Filling the section + // while it is still running would swap the generic picks out from under + // the user the moment the real ones land. + const isJobDone = + !isBrainDumpEnabled || + recommendedQuery.isError || + (recommendedQuery.data !== undefined && + (recommendedQuery.data.status !== 200 || + recommendedQuery.data.data.ready)); + const isPersonalized = personalizedProviders.length > 0; + const recommendedProviders = isPersonalized + ? personalizedProviders + : isJobDone + ? fallbackProviders(allProviders) + : []; + function handleSelect(providerId: string) { setDirection(1); setSelectedId(providerId); @@ -132,6 +152,7 @@ export function useConnectToolsPanel() { setQuery, providers, recommendedProviders, + isPersonalized, isLoading: providersQuery.isLoading, isError: providersQuery.isError, error: providersQuery.error, diff --git a/autogpt_platform/frontend/src/app/(platform)/copilot/helpers/greetingOrb.ts b/autogpt_platform/frontend/src/app/(platform)/copilot/helpers/greetingOrb.ts new file mode 100644 index 000000000000..85e66c7a8f29 --- /dev/null +++ b/autogpt_platform/frontend/src/app/(platform)/copilot/helpers/greetingOrb.ts @@ -0,0 +1,34 @@ +import type { GlassParams } from "@/components/molecules/GlassOrb/GlassSurface"; + +// The orb is one element across the greeting's arrival, not two: +// GreetingLoader renders it centered, OnboardingIntroCard renders it in +// the heading, and framer moves it between the two because they share +// this id. Both live here so neither component owns the other's values. +export const GREETING_ORB_LAYOUT_ID = "onboarding-greeting-orb"; + +export const ORB_FLIP_TRANSITION = { + type: "spring", + bounce: 0.15, + duration: 0.55, +} as const; + +// The same trip with prefers-reduced-motion on: the orb is where it +// belongs on the next frame instead of springing across the page. +export const ORB_FLIP_TRANSITION_REDUCED = { duration: 0 } as const; + +// The default glass params are tuned for the big onboarding orb; at 32px +// that much frost and distortion collapses into a flat purple ball. Light +// frost + gentle refraction keeps the drifting blobs readable this small. +export const SMALL_ORB_PARAMS: GlassParams = { + frost: 1.5, + saturation: 1.5, + tint: 0.12, + edge: 0.55, + distortion: 8, + ringWidth: 1, + ringDepth: 2, + ringDark: 0.25, +}; + +// The purple the orb's blobs blend into — the name mirrors it. +export const ORB_PURPLE = "#8a4dff"; diff --git a/autogpt_platform/frontend/src/app/api/openapi.json b/autogpt_platform/frontend/src/app/api/openapi.json index ade3a13f3abc..e4683cf824c5 100644 --- a/autogpt_platform/frontend/src/app/api/openapi.json +++ b/autogpt_platform/frontend/src/app/api/openapi.json @@ -18790,6 +18790,11 @@ "title": "Greeting Done", "default": false }, + "greeting_pending": { + "type": "boolean", + "title": "Greeting Pending", + "default": false + }, "transcript": { "anyOf": [{ "type": "string" }, { "type": "null" }], "title": "Transcript"