fix(backend): carry marketplace name/description/image to downloaded library agents - #13680
Conversation
…library agents When a marketplace agent is added to a user's library, LibraryAgent was created from the graph alone, so it showed the creator's original graph title/description and no image instead of the published marketplace values. Snapshot the StoreListingVersion name, description and first image URL onto the LibraryAgent at download time (mirroring the existing imageUrl column), and prefer them in LibraryAgent.from_db, falling back to the graph's own values for user-created agents. Closes #9879
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
/review |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (13)
WalkthroughLibrary agents now snapshot marketplace name, description, and first image when added or restored. Library responses prefer stored marketplace metadata, and searches match both snapshots and graph metadata. ChangesMarketplace metadata snapshots
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant add_store_agent_to_library
participant resolve_graph_for_library
participant add_graph_to_library
participant StoreListingVersion
participant LibraryAgent
Client->>add_store_agent_to_library: add graph to library
add_store_agent_to_library->>resolve_graph_for_library: resolve graph and listing version
resolve_graph_for_library-->>add_store_agent_to_library: GraphModel and StoreListingVersion
add_store_agent_to_library->>add_graph_to_library: pass resolved listing version
add_graph_to_library->>StoreListingVersion: read marketplace metadata
add_graph_to_library->>LibraryAgent: create or refresh snapshot
LibraryAgent-->>Client: return marketplace-preferred metadata
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (1)
58-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a Pydantic model for marketplace metadata.
_get_marketplace_metadatareturns fixed-shape structured data as a rawdict, requiring string-key indexing at every call site. Return a small Pydantic model and access typed attributes instead.As per coding guidelines, structured data should use Pydantic models over dictionaries.
🤖 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/library/_add_to_library.py` around lines 58 - 77, The _get_marketplace_metadata function returns fixed-shape marketplace data as a raw dictionary. Define a small Pydantic model for the name, description, and imageUrl fields, return that model from _get_marketplace_metadata including the missing-listing fallback, and update its call sites to use typed attributes instead of string-key indexing.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/backend/backend/api/features/library/model.py`:
- Around line 352-355: Update the marketplace snapshot fallback assignments for
name and description to use explicit None checks instead of truthiness. Preserve
intentionally empty agent.name and agent.description values, falling back to
graph.name or graph.description only when the corresponding marketplace field is
None.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py`:
- Around line 58-77: The _get_marketplace_metadata function returns fixed-shape
marketplace data as a raw dictionary. Define a small Pydantic model for the
name, description, and imageUrl fields, return that model from
_get_marketplace_metadata including the missing-listing fallback, and update its
call sites to use typed attributes instead of string-key indexing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7dd2854c-984d-47f7-8a89-34cfa1ad578f
📒 Files selected for processing (6)
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/migrations/20260727120000_add_library_agent_marketplace_metadata/migration.sqlautogpt_platform/backend/schema.prisma
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.11)
- GitHub Check: end-to-end tests
- GitHub Check: lint
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: type-check (3.12)
- GitHub Check: Check PR Status
- GitHub Check: types
- GitHub Check: Analyze (typescript)
- GitHub Check: lint
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
autogpt_platform/backend/schema.prisma
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run database migrations with 'poetry run prisma migrate dev' and 'poetry run prisma generate' after schema changes in backend
Files:
autogpt_platform/backend/schema.prisma
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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
🧠 Learnings (14)
📚 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/migrations/20260727120000_add_library_agent_marketplace_metadata/migration.sqlautogpt_platform/backend/schema.prismaautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-05-09T10:56:21.839Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13069
File: autogpt_platform/backend/migrations/20260509120000_add_chat_message_queue_status/migration.sql:1-13
Timestamp: 2026-05-09T10:56:21.839Z
Learning: In autogpt_platform/backend migrations (PostgreSQL), assume the `platform` schema is created/bootstrapped by the surrounding deployment infrastructure before Prisma migrations run. Therefore, individual migration SQL files should NOT include `CREATE SCHEMA IF NOT EXISTS "platform"` (or similar schema-creation statements); the schema should already exist, and adding it in each migration can cause unnecessary/incorrect expectations. Flag this as a review issue if present in migration `.sql` files.
Applied to files:
autogpt_platform/backend/migrations/20260727120000_add_library_agent_marketplace_metadata/migration.sql
📚 Learning: 2026-06-15T09:07:09.084Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13359
File: autogpt_platform/backend/migrations/20260615120000_add_user_workspace_folders/migration.sql:21-22
Timestamp: 2026-06-15T09:07:09.084Z
Learning: When reviewing Prisma-managed schema changes/migrations in this repository, do not recommend adding partial/expression unique indexes (e.g., `CREATE UNIQUE INDEX ... WHERE "parentId" IS NULL`) because Prisma cannot represent them in `schema.prisma`; adding them via raw SQL will create schema drift and repeated changes on future `prisma migrate dev` runs. For root-folder name uniqueness, the intended approach is the existing application-layer guard (`_root_name_taken` checks in `create_folder`/`update_folder` in `autogpt_platform/backend/backend/data/workspace_folder.py`) rather than a DB partial/index-based constraint. Prefer Prisma-supported uniqueness constraints, and if a uniqueness rule requires conditional logic, implement it at the application layer.
Applied to files:
autogpt_platform/backend/migrations/20260727120000_add_library_agent_marketplace_metadata/migration.sql
📚 Learning: 2026-07-20T23:28:10.519Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13621
File: autogpt_platform/backend/migrations/20260718120000_add_llm_catalog_runtime_tables/migration.sql:28-29
Timestamp: 2026-07-20T23:28:10.519Z
Learning: When reviewing SQL migration files under autogpt_platform/backend/migrations, do not treat DB constructs (e.g., raw-SQL partial unique indexes and CHECK constraints) as “Prisma schema drift” solely because Prisma cannot model them, as long as they follow the repo’s established workflow (`migrate deploy`/`migrate reset`) and are documented/intentional in `schema.prisma`.
For concurrency-sensitive invariants, keep the database-enforced mechanism (e.g., a partial unique index preventing simultaneous active `LlmModelMigration` rows per `sourceModelSlug`) and avoid recommending application-level check-then-insert logic, since it is vulnerable to TOCTOU races.
Applied to files:
autogpt_platform/backend/migrations/20260727120000_add_library_agent_marketplace_metadata/migration.sql
📚 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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
🔇 Additional comments (5)
autogpt_platform/backend/migrations/20260727120000_add_library_agent_marketplace_metadata/migration.sql (1)
1-7: LGTM!autogpt_platform/backend/schema.prisma (1)
611-616: LGTM!autogpt_platform/backend/backend/api/features/library/_add_to_library.py (1)
96-96: LGTM!Also applies to: 113-121, 134-136
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.py (1)
6-12: LGTM!Also applies to: 34-37, 54-57, 79-82, 107-110, 113-146
autogpt_platform/backend/backend/api/features/library/model_test.py (1)
14-15: LGTM!Also applies to: 27-28, 48-68
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13680 +/- ##
==========================================
- Coverage 76.64% 76.59% -0.05%
==========================================
Files 2715 2715
Lines 207923 207926 +3
Branches 19947 19934 -13
==========================================
- Hits 159370 159270 -100
- Misses 44153 44253 +100
- Partials 4400 4403 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![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 #13680
PR #13680 — fix(backend): carry marketplace name/description/image to downloaded library agents
Author: Abhi1992002 | Files: 6
🎯 Verdict: REQUEST_CHANGES
PR Description Quality
✅ Has Why + What + How — the snapshot-at-download rationale is clear.
What This PR Does
When a user adds a marketplace agent to their library, the library card previously showed the creator's original graph title/description/image rather than the published marketplace values. This PR snapshots the published name/description/imageUrl from the StoreListingVersion onto the LibraryAgent row at download time (two new nullable columns + read-time fallback to the graph), so the card reflects what was published. The snapshot is written on both the create and re-download/restore paths.
Specialist Findings
🛡️ Security ✅ — Traced the full download path (add_store_agent_to_library → resolve_graph_for_library → get_graph APPROVED gate → snapshot). No auth/authz boundary, SQL injection, or secrets issues; migration only adds two nullable TEXT columns.
🟡 Snapshot helper (_add_to_library.py:68) doesn't re-assert submissionStatus == APPROVED, so an owned/mismatched SLV could snapshot unreviewed text/image — but blast radius is the requesting user's own, React-escaped library card. Defense-in-depth only.
🏗️ Architecture imageUrl precedent; layering is fine.
🟠 _get_marketplace_metadata re-fetches the same StoreListingVersion already loaded by resolve_graph_for_library (_add_to_library.py:68 vs :35) — the root cause of the CI failure below.
🟠 Restore/update path can overwrite a populated snapshot with all-NULLs if the listing lookup returns nothing (_add_to_library.py:~131).
⚡ Performance StoreListingVersion PK read per add-to-library call, since the scalar name/description/imageUrls are already on the upstream-loaded slv. Zero-cost to eliminate.
🧪 Testing imageUrls branch (_add_to_library.py:78), per-field mixed fallback in from_db, and empty-string semantics are untested.
📖 Quality ✅ — Clean, well-commented change explaining why. Nits: cryptic slv local; dict[str, str|None] return indexed by magic-string keys would be safer as a TypedDict/dataclass.
📦 Product db.py:192, embeddings.py:39), so a downloaded agent may not be findable by the published title the user now sees — the exact population this PR targets.
📬 Discussion ❌ — GitHub CI is red: test (3.11) and test (3.13) fail on db_test.py::test_add_agent_to_library ("Expected find_unique to be called once. Called 2 times."). One open CodeRabbit nitpick (empty-string fallback) unaddressed; 0 human reviews.
🔎 QA ✅ — Exercised the real add-to-library path end-to-end with sentinel published values differing from the graph. Create (201), DB snapshot, list API, restore/re-download refresh, null-fallback, and negatives (401/404/422) all PASS; the rendered card shows "PUBLISHED Calculator Title", not the graph name.
🔴 Blockers
- New second
find_uniquebreaks existing test — GitHub CI red (autogpt_platform/backend/backend/api/features/library/_add_to_library.py:66) —_get_marketplace_metadataissues a secondStoreListingVersion.find_unique(), but the unmodifieddb_test.py::test_add_agent_to_libraryasserts it's called exactly once.test (3.11)andtest (3.13)fail on this PR's own change. Reusing the row already loaded byresolve_graph_for_library(withinclude: {AgentGraph: True}) fixes both the failing test and the redundant query. (Flagged by: discussion, architect, performance, quality — 4 specialists)
🟠 Should Fix
- Restore path can wipe a good snapshot with NULLs (
_add_to_library.py:~131) — if the metadata lookup returns all-None(missing listing/race), the update overwrites a previously populatedname/description/imageUrlwith NULL. Write snapshot fields only when non-null on the update path. (architect) - Search desyncs from display (
db.py:192,embeddings.py:39) — lexical filter and semantic embedding still key offgraph.name/graph.description, so an agent shown as "SEO Optimizer Pro" but graph-named "my-test-graph-v2" won't surface when searched by its visible title. COALESCE the snapshot over the graph values in both surfaces, or explicitly scope search as an accepted follow-up. (product — 2 findings) - Empty-string
orfallback (model.py:355) —agent.name or graph.name(and description) treats a published""as unset and falls back to the graph. Useagent.name if agent.name is not None else graph.nameto honor the snapshot contract, and reply to the open CodeRabbit comment. (Flagged by: architect, quality, testing, product, discussion + CodeRabbit — 5 specialists) - Untested branches (
_add_to_library.py:78,model_test.py:48) — add coverage for listing-with-empty-imageUrls(imageUrlnull while name/desc populated) and per-field mixed fallback infrom_db. Cheap to add and would have surfaced item #3. (testing)
🟡 Nice to Have
- Typed metadata return (
_add_to_library.py:58) — replacedict[str, str|None]+ magic-string keys with a TypedDict/dataclass for typo protection. (quality) - Migration is non-retroactive (
migration.sql:6) — already-downloaded agents keep NULL snapshots until re-downloaded; note in the changelog or add a backfill if retroactive correctness is desired. (architect) - Approval-gate hardening (
_add_to_library.py:68) — assertslv.agentGraphId == graph_model.id/submissionStatus == APPROVEDon the non-admin path as defense-in-depth. (security)
🔵 Nits
slvabbreviation (_add_to_library.py:66) — spell out asstore_listing_versionto match the parameter name. (quality)
Human Review Needed
NO — This is an isolated backend data-layer change with no auth/authz, credential, or trust-boundary impact; QA has proven the user-visible behavior end-to-end. It needs the CI regression fixed, not human eyes on the security boundary.
Risk Assessment
Merge risk: MEDIUM (CI currently red; additive nullable columns make the data change itself low-risk) | Rollback: EASY — additive migration, read-time fallback, no destructive changes.
CI Status
GitHub CI: ❌ FAILING — test (3.11) and test (3.13) fail on db_test.py::test_add_agent_to_library (duplicate find_unique), per the discussion specialist; test (3.12) pending. ~35 other checks green (lint, type-check, CodeQL, E2E, CLA, size/scope).
Local harness (review sandbox): frontend lint ✅, backend poetry run lint ✅, frontend types ✅, frontend test:unit ✅, frontend build ✅ — backend pytest was not run locally.
UI Testing — Variant Results
✅ local: Verified end-to-end: marketplace name/description/image snapshot onto LibraryAgent on add and on re-download restore, fall back to graph values when null, and render in the library card; all negative cases behave correctly.
✅ hosted: Snapshot of marketplace name/description/image onto downloaded library agents works correctly on create, restore, and fallback paths (verified via API, DB, and UI); one non-blocking concern that the displayed snapshot title is not searchable because library search still matches only the graph name.
- medium: The library card now displays the snapshotted marketplace name/description (agent.name or graph.name), but library search still matches only the graph's name/description, not the new LibraryAgent.name/description columns. A user who searches the exact title shown on the card ('QA REFRESHED TITLE v2') gets 0 results; the agent is only findable via the graph name they never see (search_term=Calculator -> 1 result, search_term=QA REFRESHED -> 0 results).
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 🚧 Needs work
in AutoGPT development kanban
…pshot Thread the StoreListingVersion already fetched by resolve_graph_for_library into add_graph_to_library instead of re-querying it, removing a redundant DB round-trip (and fixing db_test's single-find_unique assertion). Also preserve an intentionally-empty published name/description in from_db by falling back only when the snapshot is None, not when it is an empty string.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (1)
149-149: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse deferred interpolation for this debug log.
The f-string formats values even when debug logging is disabled. Use
%splaceholders for the complete log statement.Proposed fix
- logger.debug( - f"Added graph #{graph_model.id} v{graph_model.version} " - f"for store listing version #{store_listing_version.id} " - f"to library for user #{user_id}" - ) + logger.debug( + "Added graph #%s v%s for store listing version #%s to library for user #%s", + graph_model.id, + graph_model.version, + store_listing_version.id, + user_id, + )As per coding guidelines, use
%sfor deferred interpolation in debug logs.🤖 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/library/_add_to_library.py` at line 149, Update the debug log statement containing the store listing version message to use %s placeholders throughout, passing dynamic values as logger arguments instead of using an f-string; preserve the existing message and logging behavior.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/backend/backend/api/features/library/_add_to_library.py`:
- Around line 70-76: Validate creator-controlled image URLs before the library
snapshot is persisted, covering the imageUrls handling in the returned
LibraryAgent data. Allow only HTTPS URLs from the configured trusted-origin
allowlist, and reject or omit disallowed values so untrusted origins are never
copied into users’ LibraryAgent records.
- Around line 35-36: Update the non-admin path in get_graph so the selected
StoreListingVersion is authorized together with the graph: require approved and
non-deleted status and verify its association with that graph before returning
it. Preserve the separate admin bypass, and ensure the returned listing is the
exact authorized version used for marketplace metadata snapshots.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py`:
- Line 149: Update the debug log statement containing the store listing version
message to use %s placeholders throughout, passing dynamic values as logger
arguments instead of using an f-string; preserve the existing message and
logging behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cc21f199-579d-47bd-8021-a2900ebf8a0c
📒 Files selected for processing (6)
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- autogpt_platform/backend/backend/api/features/library/model.py
- autogpt_platform/backend/backend/api/features/library/model_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: lint
- GitHub Check: end-to-end tests
- GitHub Check: types
- GitHub Check: type-check (3.11)
- GitHub Check: lint
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (5)
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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
🧠 Learnings (12)
📚 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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-03-24T21:27:19.455Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12536
File: autogpt_platform/frontend/src/app/api/openapi.json:5732-5752
Timestamp: 2026-03-24T21:27:19.455Z
Learning: For FastAPI admin endpoints, avoid adding explicit 403/404 status-code entries in the route decorator (e.g., for the OpenAPI schema) solely to shape OpenAPI output. Keep openapi.json generation automatic, and instead document admin-only (403) and not-found (404) behavior via route docstrings. Enforce the actual behavior with automated tests rather than relying on decorator OpenAPI overrides.
Applied to files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/admin/store_admin_routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
🔇 Additional comments (5)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (2)
61-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a typed snapshot model instead of a string-keyed dict.
Callers access
marketplace["name"],marketplace["description"], andmarketplace["imageUrl"], so typos become runtime failures. Return a small Pydantic model and use attribute access.As per coding guidelines, use Pydantic models over dicts for structured data.
Source: Coding guidelines
81-84: LGTM!Also applies to: 97-97, 114-116, 121-122, 135-137
autogpt_platform/backend/backend/api/features/library/db.py (1)
1154-1157: LGTM!Also applies to: 1171-1174
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.py (1)
6-23: LGTM!Also applies to: 48-48, 61-64, 94-94, 110-113, 116-140
autogpt_platform/backend/backend/api/features/admin/store_admin_routes_test.py (1)
252-257: LGTM!Also applies to: 292-297
Library search matched only the graph's name/description, so searching the marketplace title shown on a downloaded agent's card returned no results. Also match the snapshotted LibraryAgent name/description columns.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/api/features/library/db_test.py (1)
140-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
AsyncMockfromunittest.mock.The test currently uses
mocker.AsyncMock; importAsyncMockfromunittest.mockand use it for these async mocks, as required by the backend test guidelines.Proposed change
+from unittest.mock import AsyncMock + -mock_agent_graph.return_value.find_many = mocker.AsyncMock(return_value=[]) +mock_agent_graph.return_value.find_many = AsyncMock(return_value=[]) -mock_find_many = mocker.AsyncMock(return_value=[]) +mock_find_many = AsyncMock(return_value=[]) -mock_library_agent.return_value.count = mocker.AsyncMock(return_value=0) +mock_library_agent.return_value.count = AsyncMock(return_value=0) - new=mocker.AsyncMock(return_value={}), + new=AsyncMock(return_value={}),🤖 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/library/db_test.py` around lines 140 - 150, Update the test mocks around mock_agent_graph, mock_library_agent, and _fetch_execution_counts to use AsyncMock imported from unittest.mock instead of mocker.AsyncMock. Preserve the existing return values and patch targets.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/backend/backend/api/features/library/db_test.py`:
- Around line 153-166: Add an assertion in the list_library_agents search test
for the fourth where["OR"] branch, verifying AgentGraph.is.description contains
"Published Title" with insensitive matching. Keep the existing assertions
unchanged.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/library/db_test.py`:
- Around line 140-150: Update the test mocks around mock_agent_graph,
mock_library_agent, and _fetch_execution_counts to use AsyncMock imported from
unittest.mock instead of mocker.AsyncMock. Preserve the existing return values
and patch targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f46cbe49-2982-47d5-9120-7726a78ca13e
📒 Files selected for processing (2)
autogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/db_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: check API types
- GitHub Check: end-to-end tests
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: type-check (3.11)
- GitHub Check: lint
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.11)
- GitHub Check: Seer Code Review
- GitHub Check: Check PR Status
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (python)
- GitHub Check: types
- GitHub Check: lint
🧰 Additional context used
📓 Path-based instructions (5)
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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/api/features/library/db_test.py
🧠 Learnings (12)
📚 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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.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/library/db_test.pyautogpt_platform/backend/backend/api/features/library/db.py
📚 Learning: 2026-06-22T15:12:38.754Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 13309
File: autogpt_platform/backend/backend/api/features/library/db_test.py:0-0
Timestamp: 2026-06-22T15:12:38.754Z
Learning: When constructing Prisma model instances in Significant-Gravitas/AutoGPT Python backend test code (e.g., `prisma.models.AgentNode`) for fields typed as Prisma `Json`, pass JSON-typed values as JSON-serialized strings (e.g., `json.dumps(some_dict)`) rather than passing a plain Python dict. Prisma/Pydantic expects the `Json` constructor input to be a string (`JSON input should be string` otherwise); the model will deserialize internally so the resulting attribute becomes a dict. If the field is annotated as `Json` but the constructor requires a `str`, keep the existing `# type: ignore` (or an equivalent, narrowly scoped typing adjustment) to satisfy the type checker without changing runtime behavior.
Applied to files:
autogpt_platform/backend/backend/api/features/library/db_test.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/api/features/library/db.py (1)
192-197: LGTM!Also applies to: 1159-1162, 1176-1179
|
All review threads have been addressed and resolved. Summary of changes since the first push:
On the two security findings: the non-admin add-to-library path already resolves graph access through All checks are green (backend |
|
/batch abhi-mon-prs |
…loaded library agents (#13680)
|
!deploy |
…uld-be-downloaded-with-agent
…uld-be-downloaded-with-agent
|
!deploy |
|
🚀 Deploying PR #13680 to development environment... |
|
✅ Preview environment is live (all services healthy)
Push more commits, then comment |
|
/review |
There was a problem hiding this comment.
📋 Automated Review — PR #13680
PR #13680 — fix(backend): carry marketplace name/description/image to downloaded library agents
Author: Abhi1992002 | Files: 9
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the description explains the bug (downloaded agents showed the creator's original graph title/description instead of the published marketplace values), the fix (snapshot-at-download), and the mechanism. One checklist item — the manual E2E verification — remains unchecked, but QA has independently exercised that exact path (see below).
What This PR Does
When a user downloads a marketplace agent, their library card previously showed the creator's original graph name/description/image rather than the published marketplace listing values. This PR snapshots the StoreListingVersion's name, description, and first imageUrls entry onto the LibraryAgent at download time (create + restore paths), makes from_db prefer the snapshot with a graph fallback, and extends library search to match the snapshotted fields. Two nullable TEXT columns are added via an additive, non-blocking migration.
Specialist Findings
🛡️ Security ✅ — No critical/high issues. Correctly flags that publisher-authored name/description are now persisted and rendered in the consumer's authenticated library (model.py:355), a stored-XSS surface only if the frontend renders them as unescaped HTML/Markdown — React's default text escaping mitigates this. imageUrl from imageUrls[0] (_add_to_library.py:81) is persisted without scheme validation (tracking-beacon vector). Both mirror existing marketplace display behavior and are store-approval-gated.
🟡 Confirm library card renders name/description as plain text; optionally validate the image URL scheme.
🏗️ Architecture ✅ — Follows the pre-existing imageUrl snapshot-at-download pattern; create/restore handled symmetrically; migration is additive and backwards-compatible. resolve_graph_for_library now returns the already-fetched StoreListingVersion so callers avoid a re-query (no N+1). Notes generic column naming (LibraryAgent.name/description vs. marketplaceName) and an untyped dict[str, str | None] return as maintainability polish.
🟠 Generic column names could mislead future readers (schema.prisma:612).
⚡ Performance ✅ — Net positive: the refactor eliminates a second find_unique round-trip (the same one that caused the prior CI failure). Hot-path additions are O(1) null-coalescing checks. New ILIKE '%term%' search predicates on the unindexed name/description columns are non-sargable but not a regression (AgentGraph fields already search this way) and bounded by the userId-scoped index. Migration is metadata-only DDL — non-blocking on large tables.
🧪 Testing ✅/from_db precedence (including the subtle empty-string is not None case), the search clause, and the empty-imageUrls branch all have dedicated tests. Gap: the one realistic-model integration test (db_test.py:277 test_add_agent_to_library) drives the full resolve→snapshot→create chain with a real StoreListingVersion but expected_create doesn't assert the new fields — it would pass even if the snapshot code were deleted.
🟠 Add snapshot asserts to the integration test; add a read-side image_url assertion in model_test.py.
📖 Quality ✅ — Grade A. Comments explain why (snapshot rationale), docstrings present, migration SQL annotated. Minor: helper named _marketplace_metadata (bare noun) vs. repo verb-prefix convention; untyped dict indexed with magic string keys in two write sites.
📦 Product ✅ — Fixes the reported UX issue (OPEN-2478/#9879) for the going-forward path end-to-end: title/description/image/search all covered. Notes no backfill for pre-existing downloads (they stay on graph values until re-added — by-design, non-retroactive) and that an empty published name could render a titleless card (low risk; likely validated non-empty at publish).
📬 Discussion ✅ — All 22 review threads resolved, MERGEABLE, no conflicts. Substantive bot findings were genuinely fixed in code: the empty-string or-fallback bug (is not None), search discoverability, and the prior critical double-find_unique (now a (graph, slv) tuple threaded through). Awaits human approval from @ntindle/@0ubbe (REVIEW_REQUIRED).
🔎 QA ✅ — Exercised the full download path live against a running stack. Because seed data had SLV name == graph name, QA set distinct published metadata to prove snapshot-over-graph precedence. All 10 scenarios passed: happy path, imageUrls[0] selection, DB persistence, restore-refresh, no-image edge (null image → graph wins), snapshot+graph search matching, and 401/404 negatives. No backend errors.
🟠 Should Fix
- Prior critical is resolved — verify it stays green ✅ Addressed — the second
find_uniquethat brokedb_test.py::test_add_agent_to_librarywas eliminated;resolve_graph_for_libraryreturns(graph, slv)and threads it through (db.py:1161-1185,_add_to_library.py). Per repo rule 13, the repository's GitHub CI is authoritative; the only red check (test (3.13)) is a CI cancellation/timeout while tests streamedPASSED, withtest (3.11)andtest (3.12)green on the same commit. Re-run to clear the status rollup. - Integration test doesn't assert the snapshot (
autogpt_platform/backend/backend/api/features/library/db_test.py:277) —test_add_agent_to_libraryruns the full resolve→snapshot→create chain with a realStoreListingVersionbutexpected_createonly checks pre-existing fields, so it would pass if the snapshot code were removed. Addname/description/imageUrlasserts to close the wiring gap. (Flagged by: testing) - No read-side
image_urlcoverage (autogpt_platform/backend/backend/api/features/library/model_test.py:48) —from_dbtests assert name/description precedence but never that a snapshotted image surfaces asresult.image_url. QA verified this live, but a unit assertion is cheap. (Flagged by: testing)
🟡 Nice to Have
- Rename columns to
marketplaceName/marketplaceDescription(autogpt_platform/backend/schema.prisma:612) — generic names read as always-present display fields but are NULL for user-created agents. (architect) - Type the snapshot return as a
TypedDict/dataclass (autogpt_platform/backend/backend/api/features/library/_add_to_library.py:62) — replaces magic-string-keyed dict access at both write sites and enables spreading into the Prisma payloads. (architect, quality) - Backfill or document non-retroactivity (
.../migration.sql:6) — existing downloads keep showing graph values until re-added; note this as an accepted decision. (product)
🔵 Nits
- Helper naming (
autogpt_platform/backend/backend/api/features/library/_add_to_library.py:63) —_marketplace_metadatavs. the repo's verb-prefix convention (_get_marketplace_metadata) and the PR description's own name. (quality) - Empty-name title fallback (
autogpt_platform/backend/backend/api/features/library/model.py:352) — consideragent.name or graph.namefor the user-facing title while keepingis not Nonefor description. (product)
QA Screenshots
| Screenshot | Description |
|---|---|
 |
Library page renders published titles (PUBLISHED QA Title V2, NoImage Published Title) and preview image — not the underlying graph names (This from test. Agents), proving snapshot-over-graph precedence end-to-end ✅ |
Human Review Needed
NO — This is an isolated backend data-snapshot fix with no changes to authentication, authorization, credential handling, or trust boundaries between services. The publisher-content surface is store-approval-gated and mirrors existing marketplace display behavior. Two human reviewers (@ntindle, @0ubbe) are already requested through the normal flow; this review is the independent check.
Risk Assessment
Merge risk: LOW | Rollback: EASY (additive nullable columns, no data migration, no destructive DDL)
Local Harness
✅ All 5 local checks pass: frontend lint, backend poetry run lint (85s), frontend typecheck, frontend test:unit, frontend build.
GitHub CI: 39/40 checks green on the head SHA; the single red test (3.13) is a CI cancellation/timeout (not an assertion failure) with test (3.11)/test (3.12) passing on the same commit — re-run recommended to clear the rollup.
UI Testing — Variant Results
✅ local: Snapshot-at-download works end-to-end: library agents show the published marketplace name/description/first-image (not the graph's), the restore path refreshes the snapshot, search matches both snapshot and graph values, and auth/validation negatives return 401/404 — all verified live with no backend errors.
✅ hosted: Marketplace name/description/image is correctly snapshotted onto downloaded library agents on both create and restore paths, surfaced in the API/UI, and searchable — all scenarios pass with no blocking defects.
github-project-automation
Bot
moved this from 🚧 Needs work
to 👍🏼 Mergeable
in AutoGPT development kanban
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
3 participants
Why / What / How
Why: When you add an agent from the marketplace to your library, it shows up with the creator's original graph title and description — not the title/description it was published under — and with no image. The intended behaviour (OPEN-2478 / #9879) is that a downloaded agent appears in your library just as it does in the marketplace.
What: Snapshot the marketplace listing's published name, description and image onto the
LibraryAgentat download time, and surface them in the library.How:
LibraryAgentalready had animageUrlcolumn that was never populated on marketplace downloads. This PR follows that same snapshot-at-download pattern:name/descriptioncolumns toLibraryAgent(migration included). Null for user-created agents, which keep falling back to the graph's own values.add_graph_to_librarynow reads theStoreListingVersionand writes itsname,descriptionand firstimageUrlsentry onto theLibraryAgent— on both the create and the restore-existing (soft-deleted) paths, so re-downloading also refreshes the snapshot.LibraryAgent.from_dbprefers the snapshottedname/description, falling back tograph.name/graph.description.This is scoped to the "Add to Library" download path; user-created agents (via
create_library_agent) are unaffected.Changes 🏗️
schema.prisma: addname/description(bothString?) toLibraryAgent; new migration20260727120000_add_library_agent_marketplace_metadata._add_to_library.py: new_get_marketplace_metadata()helper;add_graph_to_librarysnapshots name/description/imageUrl on create and update.library/model.py:from_dbusesagent.name or graph.nameandagent.description or graph.description._add_to_library_test.py(snapshot written on create + restore; helper edge cases) andmodel_test.py(override precedence + fallback).Checklist 📋
For code changes:
poetry run pytest backend/api/features/library/_add_to_library_test.py backend/api/features/library/model_test.py— 9 passedfrom_dbfallback path keeps user-created agents on their graph name/description