refactor(platform): consolidate 6 LD flags into 2 JSON flags - #12915
Merged
Conversation
…prices + cost limits) Matches the pattern set by copilot-tier-multipliers (just merged in #12910) — one JSON flag per configuration domain, keyed by tier or window name. - Remove: stripe-price-id-{basic,pro,max,business} + copilot-{daily,weekly}-cost-limit-microdollars. - Add: copilot-tier-stripe-prices (JSON: {tier: price_id}) + copilot-cost-limits (JSON: {daily,weekly}). - get_subscription_price_id now parses the JSON flag and looks up by tier value. - get_global_rate_limits reads the new flag via a sibling _fetch_cost_limits_flag helper (60s cache, cache_none=False); keeps ChatConfig fallbacks when the JSON is unset / non-dict / per-key invalid. - Tests rewritten to mock the new JSON shapes + cover partial / invalid / missing-key fallbacks.
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
Contributor
WalkthroughConsolidates multiple per-tier/per-window LaunchDarkly flags into two JSON-based flags: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟡 Medium Risk — Some Line OverlapThese PRs have some overlapping changes:
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 3 conflict(s), 1 medium risk, 3 low risk (out of 7 PRs with file overlap) Auto-generated on push. Ignores: |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/rate_limit_test.py (1)
575-582: Minor: considerrec.getMessage()overrec.messagefor log-record assertions.
LogRecord.messageis only populated after aFormatter.format()pass, whereasrec.getMessage()always returns the interpolated message. This works today because pytest'scaploghappens to capture the formatted message, but if the production log statement later switches to deferred%sinterpolation (which the backend coding guidelines recommend fordebuglogs), a directrec.messageread can become fragile. Not blocking.♻️ Optional tweak
- assert any("copilot-cost-limits" in rec.message for rec in caplog.records) + assert any("copilot-cost-limits" in rec.getMessage() for rec in caplog.records)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/rate_limit_test.py` around lines 575 - 582, The test reads LogRecord.message which can be empty if not formatted; update the assertion to call rec.getMessage() instead of rec.message so the interpolated log text is always returned; locate the assertion block around the call to get_global_rate_limits and the caplog usage (references: get_global_rate_limits, caplog, and the "copilot-cost-limits" log check) and change the any(...) check to use rec.getMessage() for robust log-record assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/rate_limit.py`:
- Around line 178-194: The loop that parses LD limits currently coerces values
via int(raw[key]) which accepts booleans, numeric strings, and floats; instead,
enforce strict integer types: first check that raw[key] is exactly an int (e.g.,
type(raw[key]) is int or isinstance(raw[key], int) and not isinstance(raw[key],
bool)), log the same warning and continue when the type is wrong, then check for
negativity and only then assign into parsed; reference the variables 'raw',
'parsed' and the 'logger' warnings in the for key in ("daily", "weekly") block
to locate and update the code.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/rate_limit_test.py`:
- Around line 575-582: The test reads LogRecord.message which can be empty if
not formatted; update the assertion to call rec.getMessage() instead of
rec.message so the interpolated log text is always returned; locate the
assertion block around the call to get_global_rate_limits and the caplog usage
(references: get_global_rate_limits, caplog, and the "copilot-cost-limits" log
check) and change the any(...) check to use rec.getMessage() for robust
log-record assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 494b3783-7074-4870-9117-9d220d4b696e
📒 Files selected for processing (5)
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/data/credit_subscription_test.pyautogpt_platform/backend/backend/util/feature_flag.py
📜 Review details
🧰 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/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
🧠 Learnings (25)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-04-22T12:26:42.571Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py`, `_resolve_sdk_model_for_request`: when a per-user LaunchDarkly model value fails `_normalize_model_name` (e.g. a `moonshotai/kimi-*` slug in direct-Anthropic mode), the fallback must be tier-specific — `config.thinking_advanced_model` for advanced tier, `config.thinking_standard_model` for standard tier — NOT the generic `_resolve_sdk_model()` (which is standard-only and returns None under subscription mode). If the tier-specific config default also fails `_normalize_model_name`, re-raise the original LD error; this is a deployment-level misconfiguration that `model_validator` should have caught at startup. Established in PR `#12881` commit 637d2fef5.
📚 Learning: 2026-03-13T15:49:44.961Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-13T15:49:44.961Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, the original per-session token window (with a TTL-based reset) was replaced with fixed daily and weekly windows. `resets_at` is now derived from `_daily_reset_time()` (midnight UTC) and `_weekly_reset_time()` (next Monday 00:00 UTC) — deterministic fixed-boundary calculations that require no Redis TTL introspection.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-03-15T15:29:20.889Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-15T15:29:20.889Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, the daily and weekly Redis keys encode the current date/week directly in the key name (e.g., `copilot:usage:daily:{user_id}:{YYYY-MM-DD}` and `copilot:usage:weekly:{user_id}:{year}-W{week}`). This means a new key is naturally created at each window boundary, so `resets_at` (derived from `_daily_reset_time()` / `_weekly_reset_time()`) is always accurate without any Redis TTL introspection — the key rotation and reset-time calculation are inherently synchronized.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-03-15T23:39:39.754Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-15T23:39:39.754Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, `record_token_usage` uses the same helper functions (`_daily_reset_time()` / `_weekly_reset_time()`) to compute both `resets_at` (the reset timestamp returned to callers) and the Redis key `expire` seconds. This single-source-of-truth design guarantees that the reported reset times and the actual Redis TTLs are always in sync — there is no separate TTL constant that could diverge from the calendar-boundary calculation.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-03T13:50:29.037Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/backend/api/external/v2/rate_limit.py:24-56
Timestamp: 2026-04-03T13:50:29.037Z
Learning: In `autogpt_platform/backend/backend/api/external/v2/rate_limit.py`, the `RateLimiter` class uses in-process (per-worker) memory for sliding-window rate limiting. This is intentionally documented as a known limitation via WARNING comments in the module and class docstrings. A full Redis-backed migration (using ZADD/ZREMRANGEBYSCORE/ZCARD with TTL/Lua for atomic multi-replica enforcement) is deferred to a later PR. Do not re-flag the in-memory implementation as a blocking bug — the limitation is documented and accepted for the initial v2 external API release.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-16T17:00:02.827Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12439
File: autogpt_platform/backend/backend/blocks/autogpt_copilot.py:0-0
Timestamp: 2026-03-16T17:00:02.827Z
Learning: In autogpt_platform/backend/backend/blocks/autogpt_copilot.py, the recursion guard uses two module-level ContextVars: `_copilot_recursion_depth` (tracks current nesting depth) and `_copilot_recursion_limit` (stores the chain-wide ceiling). On the first invocation, `_copilot_recursion_limit` is set to `max_recursion_depth`; nested calls use `min(inherited_limit, max_recursion_depth)`, so they can only lower the cap, never raise it. The entry/exit logic is extracted into module-level helper functions. This is the approved pattern for preventing runaway sub-agent recursion in AutogptCopilotBlock (PR `#12439`, commits 348e9f8e2 and 3b70f61b1).
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-12T14:42:40.552Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:141-170
Timestamp: 2026-03-12T14:42:40.552Z
Learning: In Significant-Gravitas/AutoGPT, `check_rate_limit` in `autogpt_platform/backend/backend/copilot/rate_limit.py` is intentionally a "pre-turn soft check" (not a hard atomic reservation). Because LLM token counts are unknown before generation completes, a strict check-and-reserve is impractical. The TOCTOU race (two concurrent turns both passing the pre-check and both committing via `record_token_usage`) is an accepted trade-off. If stricter enforcement is ever needed, the approach is a Lua script doing GET+INCRBY atomically in Redis.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-23T13:53:40.315Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12894
File: autogpt_platform/backend/backend/data/block_cost_config.py:271-277
Timestamp: 2026-04-23T13:53:40.315Z
Learning: In `autogpt_platform/backend/backend/data/block_cost_config.py`, `compute_token_credits()` intentionally returns `MODEL_COST[model]` (the flat tier) on pre-flight (when `stats is None`) for the `TOKENS` billing path. Returning 0 pre-flight would allow a zero-balance user to bypass the credit gate and trigger an LLM call, with the insufficient-balance error only surfacing post-flight (a billing leak). The overcharge concern (actual token cost < MODEL_COST estimate) is handled by `_charge_reconciled_usage_sync` in `autogpt_platform/backend/backend/executor/billing.py`, which issues a negative-delta refund via `spend_credits(cost=negative)` when real usage falls below the pre-flight estimate. Do NOT flag the MODEL_COST pre-flight floor in this function as an overcharge bug; the refund path covers it.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-04-23T00:07:27.117Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 0
File: :0-0
Timestamp: 2026-04-23T00:07:27.117Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/openrouter_cost.py`, the `openrouter-cost-reconcile` Langfuse event carries `cost_source` ("openrouter" for authoritative OpenRouter-resolved cost, "fallback" for rate-card fallback) and `resolved_generation_id_count` alongside the reconciled cost and token/model/provider metadata. This lets operators distinguish authoritative reconciliations from fallbacks in Langfuse. Established in PR `#12889` commit 5ce3d0388.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_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/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_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/copilot/rate_limit.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-17T10:57:12.953Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Applied to files:
autogpt_platform/backend/backend/util/feature_flag.py
📚 Learning: 2026-04-21T04:36:19.755Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit_subscription_test.py:1119-1122
Timestamp: 2026-04-21T04:36:19.755Z
Learning: In `autogpt_platform/backend/backend/data/credit_subscription_test.py` (and related subscription test files), test mocks for the user object returned by `get_user_by_id` should use snake_case `subscription_tier` (not camelCase `subscriptionTier`). This is because `get_user_by_id` (defined in `backend/data/user.py`) returns `backend.data.model.User` — a Pydantic application model with `subscription_tier: SubscriptionTier` — not a raw Prisma model. Production code in `backend/data/credit.py` reads `user.subscription_tier` from that Pydantic model. Do NOT flag `mock_user.subscription_tier = ...` as incorrect in these tests.
Applied to files:
autogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-04-22T12:26:42.571Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-04-22T12:26:42.571Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py`, `_resolve_sdk_model_for_request`: when a per-user LaunchDarkly model value fails `_normalize_model_name` (e.g. a `moonshotai/kimi-*` slug in direct-Anthropic mode), the fallback must be tier-specific — `config.thinking_advanced_model` for advanced tier, `config.thinking_standard_model` for standard tier — NOT the generic `_resolve_sdk_model()` (which is standard-only and returns None under subscription mode). If the tier-specific config default also fails `_normalize_model_name`, re-raise the original LD error; this is a deployment-level misconfiguration that `model_validator` should have caught at startup. Established in PR `#12881` commit 637d2fef5.
Applied to files:
autogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-04-21T04:35:34.710Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit.py:1584-1584
Timestamp: 2026-04-21T04:35:34.710Z
Learning: When reviewing this codebase, don’t flag snake_case attribute names (e.g., `subscription_tier`, `stripe_customer_id`, `top_up_config`) on the app-layer Pydantic `User` model as “wrong” field names. These are correct for the app-layer model and are expected to be mapped from the Prisma-layer camelCase fields (e.g., `subscriptionTier`, `stripeCustomerId`) inside methods like `User.from_db()`. Only Prisma-returned/raw objects would use camelCase, but functions like `get_user_by_id(user_id: str)` are expected to return the Pydantic app-layer model.
Applied to files:
autogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-04-22T04:01:32.723Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12876
File: autogpt_platform/backend/backend/copilot/tools/helpers.py:0-0
Timestamp: 2026-04-22T04:01:32.723Z
Learning: In `autogpt_platform/backend/backend/copilot/tools/helpers.py`, the `run_block` copilot path charges ONLY the user credit wallet (via `spend_credits` / `_charge_block_credits`). The microdollar rate-limit counter (`record_cost_usage`) is NOT incremented for `run_block` block executions — the `_record_block_microdollar_cost` helper was explicitly reverted (commit 16ae0f7b5, PR `#12876`) to avoid double-accounting. Do NOT flag missing microdollar recording in `execute_block` as a bug; the credit wallet is the sole billing mechanism for copilot `run_block` calls.
Applied to files:
autogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.py
📚 Learning: 2026-04-21T04:36:19.755Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit_subscription_test.py:1119-1122
Timestamp: 2026-04-21T04:36:19.755Z
Learning: In subscription-related test files under autogpt_platform/backend/backend/data/, when mocking the return value of get_user_by_id, use snake_case `subscription_tier` (e.g., `mock_user.subscription_tier = ...`) rather than camelCase `subscriptionTier`. This is because get_user_by_id returns `backend.data.model.User`, a Pydantic model that defines `subscription_tier: SubscriptionTier`, and production code (e.g., backend/data/credit.py) reads `user.subscription_tier`. Do not flag mocks that set `mock_user.subscription_tier` as incorrect in these tests.
Applied to files:
autogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-19T15:10:53.815Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:53.815Z
Learning: In Python unittest.mock, the correct patch target depends on whether an import is eager (module-level) or lazy (inside a function/branch):
- **Module-level import** (`from foo.bar import baz` at top of file): patch where the name is used, e.g. `patch("mymodule.baz")`.
- **Lazy import** (`from foo.bar import baz` inside a function/branch, executed at call time): patch the source module, e.g. `patch("foo.bar.baz")`, because the fresh `from ... import` at call time will look up the (now-patched) name in the source module's dict.
This pattern appears in `autogpt_platform/backend/backend/copilot/tools/helpers.py` where `simulate_block` is lazily imported inside the `if dry_run:` block, making `patch("backend.executor.simulator.simulate_block")` the correct target in tests.
Applied to files:
autogpt_platform/backend/backend/data/credit_subscription_test.py
🔇 Additional comments (5)
autogpt_platform/backend/backend/util/feature_flag.py (1)
45-47: Flag-key consolidation in enum looks correct.The new enum keys are consistent with the JSON-flag migration and keep call-sites centralized.
autogpt_platform/backend/backend/data/credit.py (1)
1943-1967: JSON flag parsing and fallback behavior are solid here.The new lookup path handles unset/malformed payloads safely and preserves non-cached
Nonebehavior for transient LD issues.autogpt_platform/backend/backend/data/credit_subscription_test.py (1)
948-1107: Good expansion of LD-edge-case coverage.These test changes correctly validate dict payload handling, invalid payload fallback, and the non-caching of transient
Noneresponses.autogpt_platform/backend/backend/copilot/rate_limit_test.py (2)
471-636: LGTM — thorough coverage of the newcopilot-cost-limitsJSON parsing paths.The new
TestGetGlobalRateLimitsCostLimitsFlagclass exercises all the behaviors the PR objectives call out: unset flag → config defaults, both keys honored, missing-key partial fallback, non-dict payload with warning referencingcopilot-cost-limits, fully invalid values, and the critical partial-invalid case where a valid key survives when the sibling key is broken. Autouse cache-clear (lines 482-485) correctly covers both_fetch_tier_multipliers_flagand_fetch_cost_limits_flagto prevent cross-test leakage.
910-928: LGTM —_ld_side_effecthelpers consistently migrated to the new dict shape.All three
_clear_flag_cachefixtures now clear_fetch_cost_limits_flag, and the three_ld_side_effectvariants return{"daily": ..., "weekly": ...}for thecost-limitsflag key instead of raw ints keyed by substring. Substring match on"cost-limits"correctly targetscopilot-cost-limitswithout colliding with thetier-multipliersbranch.Also applies to: 1089-1100, 1305-1317
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #12915 +/- ##
==========================================
+ Coverage 68.21% 68.22% +0.01%
==========================================
Files 1959 1959
Lines 149934 150073 +139
Branches 15606 15617 +11
==========================================
+ Hits 102273 102391 +118
- Misses 44627 44646 +19
- Partials 3034 3036 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…n_price_id cache size
- copilot-cost-limits parser now rejects bool/str/float/list values via isinstance(v, int) and not isinstance(v, bool). Matches docstring promise ("non-int values are skipped") — int() was silently coercing True/"100"/1.9 into a rate-limit cap (CodeRabbit finding).
- get_subscription_price_id cache bumped from maxsize=1 → maxsize=8 (Sentry finding). The function takes a tier argument; maxsize=1 was thrashing the cache on concurrent calls for different tiers. 8 covers the 5 tier enum values with slack.
- Added a parametrized test asserting bool/str/float/list/None all default-fallback instead of coercing.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
autogpt_platform/backend/backend/copilot/rate_limit.py (1)
785-811: Behavioral change worth flagging in the docstring: cost-limits are now evaluated system-wide.The previous per-flag evaluation passed
user_idas LD context, which supported per-user/per-cohort targeting rules oncopilot-daily-cost-limit-microdollars/copilot-weekly-cost-limit-microdollars. After this refactor,_fetch_cost_limits_flag()evaluatescopilot-cost-limitswith the fixed"system"context (and is cached globally withmaxsize=1), so any targeting rules from the legacy flags cannot be reproduced on the new flag. That appears consistent with the tier-multipliers pattern and is presumably intentional, but theuser_idparameter docstring ("User ID for LD flag evaluation context") is now only accurate for the tier lookup — worth a quick doc tweak so operators migrating the LD flags don't expect per-user targeting oncopilot-cost-limits.📝 Suggested doc clarification
Args: - user_id: User ID for LD flag evaluation context. + user_id: User ID used to resolve the per-user subscription tier (for + the tier multiplier). The ``copilot-cost-limits`` flag itself is + evaluated system-wide and does not support per-user targeting. config_daily: Fallback daily cost limit (microdollars) from ChatConfig. config_weekly: Fallback weekly cost limit (microdollars) from ChatConfig.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/rate_limit.py` around lines 785 - 811, Update the get_global_rate_limits docstring to reflect that cost limits are now evaluated system-wide (via _fetch_cost_limits_flag which uses a fixed "system" context and is globally cached with maxsize=1) and that the user_id parameter is only used for the tier lookup (not for evaluating copilot-cost-limits); mention the legacy per-user/per-cohort targeting on the old copilot-daily-cost-limit-microdollars / copilot-weekly-cost-limit-microdollars flags is not reproduced by the new flag so operators should adjust LD targeting accordingly.autogpt_platform/backend/backend/data/credit_subscription_test.py (2)
1119-1139: Consider strengthening the assertion to cover per-tier cache independence.The test validates that a transient
NoneforPROis not cached, then succeeds on a subsequent call. With the cachemaxsizeincreased to 8 (per commit summary) to handle different tier arguments without thrashing, it would be worth adding a small follow-on assertion that a successful lookup for one tier doesn't evict a concurrent lookup for another tier (e.g., PRO then MAX within the same mocked dict). Optional — current test already covers thecache_none=Falsecontract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/data/credit_subscription_test.py` around lines 1119 - 1139, Extend test_get_subscription_price_id_none_not_cached to also assert per-tier cache independence by calling get_subscription_price_id for a different tier (e.g., SubscriptionTier.MAX) under the same patched get_feature_flag_value mock and verifying it returns its corresponding price id and increments mock_ld.call_count; specifically, after confirming PRO first returned None then succeeded, call await get_subscription_price_id(SubscriptionTier.MAX) (with the mock returning a dict containing both "PRO" and "MAX" price keys) and assert the MAX result equals its expected "price_max_monthly" and that mock_ld.call_count reflects an additional call so that caching is tracked per-tier rather than globally.
1011-1029: Nit: redundantcache_clear()inside the loop.
get_subscription_price_idis keyed on thetierargument, so each iteration queries a distinct cache key — clearing between iterations isn't required for correctness (the outer clears before/after thewithblock are sufficient). Harmless, but can be simplified:♻️ Proposed simplification
for tier in ( SubscriptionTier.BASIC, SubscriptionTier.PRO, SubscriptionTier.MAX, SubscriptionTier.BUSINESS, SubscriptionTier.ENTERPRISE, ): assert await get_subscription_price_id(tier) is None - get_subscription_price_id.cache_clear() # type: ignore[attr-defined]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/data/credit_subscription_test.py` around lines 1011 - 1029, Remove the redundant get_subscription_price_id.cache_clear() call inside the loop in test_get_subscription_price_id_empty_flag_returns_none; the cache is keyed by the tier argument so the outer cache_clear() before the with block (and after, if present) is sufficient—leave the initial cache_clear() calls and delete the per-iteration cache_clear() to simplify the test while keeping calls to get_subscription_price_id and the AsyncMock patch intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/rate_limit.py`:
- Around line 785-811: Update the get_global_rate_limits docstring to reflect
that cost limits are now evaluated system-wide (via _fetch_cost_limits_flag
which uses a fixed "system" context and is globally cached with maxsize=1) and
that the user_id parameter is only used for the tier lookup (not for evaluating
copilot-cost-limits); mention the legacy per-user/per-cohort targeting on the
old copilot-daily-cost-limit-microdollars /
copilot-weekly-cost-limit-microdollars flags is not reproduced by the new flag
so operators should adjust LD targeting accordingly.
In `@autogpt_platform/backend/backend/data/credit_subscription_test.py`:
- Around line 1119-1139: Extend test_get_subscription_price_id_none_not_cached
to also assert per-tier cache independence by calling get_subscription_price_id
for a different tier (e.g., SubscriptionTier.MAX) under the same patched
get_feature_flag_value mock and verifying it returns its corresponding price id
and increments mock_ld.call_count; specifically, after confirming PRO first
returned None then succeeded, call await
get_subscription_price_id(SubscriptionTier.MAX) (with the mock returning a dict
containing both "PRO" and "MAX" price keys) and assert the MAX result equals its
expected "price_max_monthly" and that mock_ld.call_count reflects an additional
call so that caching is tracked per-tier rather than globally.
- Around line 1011-1029: Remove the redundant
get_subscription_price_id.cache_clear() call inside the loop in
test_get_subscription_price_id_empty_flag_returns_none; the cache is keyed by
the tier argument so the outer cache_clear() before the with block (and after,
if present) is sufficient—leave the initial cache_clear() calls and delete the
per-iteration cache_clear() to simplify the test while keeping calls to
get_subscription_price_id and the AsyncMock patch intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bad305e7-44e8-460b-96b6-96921ca9acec
📒 Files selected for processing (4)
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- autogpt_platform/backend/backend/data/credit.py
- autogpt_platform/backend/backend/copilot/rate_limit_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
- GitHub Check: end-to-end tests
- 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/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/credit_subscription_test.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/credit_subscription_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/data/credit_subscription_test.py
🧠 Learnings (27)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-04-22T12:26:42.571Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py`, `_resolve_sdk_model_for_request`: when a per-user LaunchDarkly model value fails `_normalize_model_name` (e.g. a `moonshotai/kimi-*` slug in direct-Anthropic mode), the fallback must be tier-specific — `config.thinking_advanced_model` for advanced tier, `config.thinking_standard_model` for standard tier — NOT the generic `_resolve_sdk_model()` (which is standard-only and returns None under subscription mode). If the tier-specific config default also fails `_normalize_model_name`, re-raise the original LD error; this is a deployment-level misconfiguration that `model_validator` should have caught at startup. Established in PR `#12881` commit 637d2fef5.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
📚 Learning: 2026-03-13T15:49:44.961Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-13T15:49:44.961Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, the original per-session token window (with a TTL-based reset) was replaced with fixed daily and weekly windows. `resets_at` is now derived from `_daily_reset_time()` (midnight UTC) and `_weekly_reset_time()` (next Monday 00:00 UTC) — deterministic fixed-boundary calculations that require no Redis TTL introspection.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-15T23:39:39.754Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-15T23:39:39.754Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, `record_token_usage` uses the same helper functions (`_daily_reset_time()` / `_weekly_reset_time()`) to compute both `resets_at` (the reset timestamp returned to callers) and the Redis key `expire` seconds. This single-source-of-truth design guarantees that the reported reset times and the actual Redis TTLs are always in sync — there is no separate TTL constant that could diverge from the calendar-boundary calculation.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-15T15:29:20.889Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-15T15:29:20.889Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, the daily and weekly Redis keys encode the current date/week directly in the key name (e.g., `copilot:usage:daily:{user_id}:{YYYY-MM-DD}` and `copilot:usage:weekly:{user_id}:{year}-W{week}`). This means a new key is naturally created at each window boundary, so `resets_at` (derived from `_daily_reset_time()` / `_weekly_reset_time()`) is always accurate without any Redis TTL introspection — the key rotation and reset-time calculation are inherently synchronized.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-16T17:00:02.827Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12439
File: autogpt_platform/backend/backend/blocks/autogpt_copilot.py:0-0
Timestamp: 2026-03-16T17:00:02.827Z
Learning: In autogpt_platform/backend/backend/blocks/autogpt_copilot.py, the recursion guard uses two module-level ContextVars: `_copilot_recursion_depth` (tracks current nesting depth) and `_copilot_recursion_limit` (stores the chain-wide ceiling). On the first invocation, `_copilot_recursion_limit` is set to `max_recursion_depth`; nested calls use `min(inherited_limit, max_recursion_depth)`, so they can only lower the cap, never raise it. The entry/exit logic is extracted into module-level helper functions. This is the approved pattern for preventing runaway sub-agent recursion in AutogptCopilotBlock (PR `#12439`, commits 348e9f8e2 and 3b70f61b1).
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-03T13:50:29.037Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/backend/api/external/v2/rate_limit.py:24-56
Timestamp: 2026-04-03T13:50:29.037Z
Learning: In `autogpt_platform/backend/backend/api/external/v2/rate_limit.py`, the `RateLimiter` class uses in-process (per-worker) memory for sliding-window rate limiting. This is intentionally documented as a known limitation via WARNING comments in the module and class docstrings. A full Redis-backed migration (using ZADD/ZREMRANGEBYSCORE/ZCARD with TTL/Lua for atomic multi-replica enforcement) is deferred to a later PR. Do not re-flag the in-memory implementation as a blocking bug — the limitation is documented and accepted for the initial v2 external API release.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-23T13:53:40.315Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12894
File: autogpt_platform/backend/backend/data/block_cost_config.py:271-277
Timestamp: 2026-04-23T13:53:40.315Z
Learning: In `autogpt_platform/backend/backend/data/block_cost_config.py`, `compute_token_credits()` intentionally returns `MODEL_COST[model]` (the flat tier) on pre-flight (when `stats is None`) for the `TOKENS` billing path. Returning 0 pre-flight would allow a zero-balance user to bypass the credit gate and trigger an LLM call, with the insufficient-balance error only surfacing post-flight (a billing leak). The overcharge concern (actual token cost < MODEL_COST estimate) is handled by `_charge_reconciled_usage_sync` in `autogpt_platform/backend/backend/executor/billing.py`, which issues a negative-delta refund via `spend_credits(cost=negative)` when real usage falls below the pre-flight estimate. Do NOT flag the MODEL_COST pre-flight floor in this function as an overcharge bug; the refund path covers it.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-12T14:42:40.552Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:141-170
Timestamp: 2026-03-12T14:42:40.552Z
Learning: In Significant-Gravitas/AutoGPT, `check_rate_limit` in `autogpt_platform/backend/backend/copilot/rate_limit.py` is intentionally a "pre-turn soft check" (not a hard atomic reservation). Because LLM token counts are unknown before generation completes, a strict check-and-reserve is impractical. The TOCTOU race (two concurrent turns both passing the pre-check and both committing via `record_token_usage`) is an accepted trade-off. If stricter enforcement is ever needed, the approach is a Lua script doing GET+INCRBY atomically in Redis.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-22T12:26:42.571Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-04-22T12:26:42.571Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py`, `_resolve_sdk_model_for_request`: when a per-user LaunchDarkly model value fails `_normalize_model_name` (e.g. a `moonshotai/kimi-*` slug in direct-Anthropic mode), the fallback must be tier-specific — `config.thinking_advanced_model` for advanced tier, `config.thinking_standard_model` for standard tier — NOT the generic `_resolve_sdk_model()` (which is standard-only and returns None under subscription mode). If the tier-specific config default also fails `_normalize_model_name`, re-raise the original LD error; this is a deployment-level misconfiguration that `model_validator` should have caught at startup. Established in PR `#12881` commit 637d2fef5.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-10T08:39:22.025Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*.py : Use `max(0, value)` guards for computed values that should never be negative
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*.py : Do not use linter suppressors — no `# type: ignore`, `# noqa`, `# pyright: ignore`; fix the type/code instead
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-02T13:16:03.050Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12649
File: .gitleaks.toml:34-35
Timestamp: 2026-04-02T13:16:03.050Z
Learning: In Significant-Gravitas/AutoGPT, the `.gitleaks.toml` allowlist regex `Llama-\d.*Instruct` is intentionally narrow. Only `Llama-X-...-Instruct-FP8` style model name enum values in `autogpt_platform/backend/backend/blocks/llm.py` actually trigger gitleaks' `generic-api-key` rule due to high entropy in the FP8-quantization suffix. Lowercase variants (e.g., `llama-3.3-70b-versatile`) and Mistral model names do not reach gitleaks' entropy/pattern thresholds, so broadening the pattern is unnecessary. Do not flag this allowlist as too narrow.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-08T17:27:45.740Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-08T17:27:45.740Z
Learning: Applies to autogpt_platform/**/data/**/*.py : For changes touching `data/*.py`, validate user ID checks or explain why not needed
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-24T21:27:22.326Z
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:22.326Z
Learning: Repo: Significant-Gravitas/AutoGPT — Preference: Do not add explicit 403/404 entries to FastAPI route decorators for admin endpoints just to influence OpenAPI. Keep openapi.json autogenerated and use route docstrings to document admin-only (403) and not-found (404) behavior; rely on tests for enforcement. File context: autogpt_platform/backend/backend/api/features/admin/store_admin_routes.py. PR `#12536`.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_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/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_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/copilot/rate_limit.pyautogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-04-21T04:36:19.755Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit_subscription_test.py:1119-1122
Timestamp: 2026-04-21T04:36:19.755Z
Learning: In subscription-related test files under autogpt_platform/backend/backend/data/, when mocking the return value of get_user_by_id, use snake_case `subscription_tier` (e.g., `mock_user.subscription_tier = ...`) rather than camelCase `subscriptionTier`. This is because get_user_by_id returns `backend.data.model.User`, a Pydantic model that defines `subscription_tier: SubscriptionTier`, and production code (e.g., backend/data/credit.py) reads `user.subscription_tier`. Do not flag mocks that set `mock_user.subscription_tier` as incorrect in these tests.
Applied to files:
autogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-03-19T15:10:53.815Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:53.815Z
Learning: In Python unittest.mock, the correct patch target depends on whether an import is eager (module-level) or lazy (inside a function/branch):
- **Module-level import** (`from foo.bar import baz` at top of file): patch where the name is used, e.g. `patch("mymodule.baz")`.
- **Lazy import** (`from foo.bar import baz` inside a function/branch, executed at call time): patch the source module, e.g. `patch("foo.bar.baz")`, because the fresh `from ... import` at call time will look up the (now-patched) name in the source module's dict.
This pattern appears in `autogpt_platform/backend/backend/copilot/tools/helpers.py` where `simulate_block` is lazily imported inside the `if dry_run:` block, making `patch("backend.executor.simulator.simulate_block")` the correct target in tests.
Applied to files:
autogpt_platform/backend/backend/data/credit_subscription_test.py
📚 Learning: 2026-04-21T04:35:34.710Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit.py:1584-1584
Timestamp: 2026-04-21T04:35:34.710Z
Learning: When reviewing this codebase, don’t flag snake_case attribute names (e.g., `subscription_tier`, `stripe_customer_id`, `top_up_config`) on the app-layer Pydantic `User` model as “wrong” field names. These are correct for the app-layer model and are expected to be mapped from the Prisma-layer camelCase fields (e.g., `subscriptionTier`, `stripeCustomerId`) inside methods like `User.from_db()`. Only Prisma-returned/raw objects would use camelCase, but functions like `get_user_by_id(user_id: str)` are expected to return the Pydantic app-layer model.
Applied to files:
autogpt_platform/backend/backend/data/credit_subscription_test.py
🔇 Additional comments (2)
autogpt_platform/backend/backend/copilot/rate_limit.py (1)
151-197: LGTM — strict int parsing correctly rejects booleans and coercions.
_fetch_cost_limits_flagmirrors_fetch_tier_multipliers_flagnicely: non-dict payload short-circuits, per-key failures are independent, and theisinstance(value, bool) or not isinstance(value, int)guard correctly rejectsTrue/"100"/1.9/[1,2](addressing the prior review feedback). Returningparsed or Nonealso keeps the "no valid overrides" path indistinguishable from "flag unset," which the caller handles viaoverride or {}.autogpt_platform/backend/backend/data/credit_subscription_test.py (1)
949-1116: Test coverage for the new JSON flag shape looks thorough and correct.The updated/added tests exercise the new
copilot-tier-stripe-pricescontract well:
- happy paths for PRO/MAX/BASIC with tier-keyed dicts,
- ENTERPRISE (key always absent),
- empty dict, partial dict, empty-string value, non-string/null value,
- non-dict payload (with the warning-message assertion pinned to the flag name),
- transient LD
None.Mock target (
backend.data.credit.get_feature_flag_value) correctly patches where the symbol is used post-refactor, andcache_clear()is called between scenarios to prevent cross-test bleed. No issues found.
This was referenced Apr 24, 2026
github-project-automation
Bot
moved this from 🆕 Needs initial review
to ✅ Done
in AutoGPT development kanban
5 tasks
majdyz
added a commit
that referenced
this pull request
Apr 25, 2026
… 1 JSON flag (#12917) ## What Replaces 4 string-valued LaunchDarkly flags with a single JSON-valued flag for copilot model routing: - ~~`copilot-fast-standard-model`~~ - ~~`copilot-fast-advanced-model`~~ - ~~`copilot-thinking-standard-model`~~ - ~~`copilot-thinking-advanced-model`~~ **New:** `copilot-model-routing` (JSON), keyed `{mode: {tier: model}}`: ```json { "fast": { "standard": "anthropic/claude-sonnet-4-6", "advanced": "anthropic/claude-opus-4-6" }, "thinking": { "standard": "moonshotai/kimi-k2.6", "advanced": "anthropic/claude-opus-4-6" } } ``` ## Why Same pattern as the sibling consolidation in #12915 (pricing / cost-limits flags) and the merged #12910 (tier-multipliers): - One flag per config domain — less LD UI clutter, easier audit trail. - Atomic updates — rotating fast.standard + thinking.standard is a single save. - Fewer LD entities to name, version, target, explain. - Mirrors the now-uniform copilot-* JSON-flag shape. ## How - `backend/util/feature_flag.py`: drop the four `COPILOT_*_MODEL` enum values, add `COPILOT_MODEL_ROUTING`. - `backend/copilot/model_router.py`: rewrite `resolve_model` to fetch the JSON flag once per call and walk `payload[mode][tier]`. Missing mode, missing tier-within-mode, non-string cell value, non-dict payload, or LD failure all fall back to the corresponding `ChatConfig` default (same user-visible semantics as before). `_FLAG_BY_CELL` removed entirely; `_config_default` / `ModelMode` / `ModelTier` unchanged. - Per-user LD targeting preserved — cohorts can still receive different routing. - No caching added (preserves existing uncached behaviour). - Docstring references in `copilot/config.py` + `copilot/sdk/service.py` updated to point at the new nested key path; one docstring in `service_test.py` likewise. ## Operator action required BEFORE merging This PR removes 4 LD flags and introduces 1 replacement. 1. In LaunchDarkly, create `copilot-model-routing` (type: **JSON**, server-side only). Default variation = union of the current four string flags, shaped as: ```json { "fast": { "standard": "<current copilot-fast-standard-model>", "advanced": "<current copilot-fast-advanced-model>" }, "thinking": { "standard": "<current copilot-thinking-standard-model>", "advanced": "<current copilot-thinking-advanced-model>" } } ``` Omit any cell that's currently unset (its `ChatConfig` default will be used). 2. Merge this PR. 3. After deploy + smoke, delete the four legacy flags: - `copilot-fast-standard-model` - `copilot-fast-advanced-model` - `copilot-thinking-standard-model` - `copilot-thinking-advanced-model` ## Testing - `backend/copilot/model_router_test.py` rewritten — 27 tests pass: - LD unset / `None` payload → fallback for every cell. - Full JSON → each cell maps to its value (parametrized). - Partial JSON (missing mode, missing tier-within-mode, mode value not a dict). - Non-dict payloads (str / list / int / bool) → fallback + warning. - Non-string cell values (number, list, bool, dict) → fallback + 'non-string' warning. - Empty-string cell → fallback + 'empty string' warning (not 'non-string'). - LD raises → fallback + warning with `exc_info`. - `user_id=None` → skip LD entirely. - Single-LD-call regression guard against re-introducing per-cell flag fan-out. - `backend/copilot/sdk/service_test.py`: 61 tests still pass (it mocks `_resolve_thinking_model_for_user`, so the inner flag change is transparent). - `black --check` / `ruff check` / `isort --check` all clean. ## Sibling - #12915 — same consolidation pattern for stripe-price / cost-limits flags. ## Checklist - [x] I have read the project's contributing guide. - [x] I have clearly described what this PR changes and why. - [x] My code follows the style guidelines of this project. - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] New and existing unit tests pass locally with my changes (CI will confirm).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Consolidates two groups of LaunchDarkly flags into single JSON-valued flags, matching the pattern established by
copilot-tier-multipliers(merged in #12910):Stripe prices — 4 string flags → 1 JSON flag:
stripe-price-id-basic-pro-max-businesscopilot-tier-stripe-prices(JSON){ "PRO": "price_xxx", "MAX": "price_yyy" }Cost limits — 2 number flags → 1 JSON flag:
copilot-daily-cost-limit-microdollarscopilot-weekly-cost-limit-microdollarscopilot-cost-limits(JSON){ "daily": 625000, "weekly": 3125000 }Why
copilot-tier-multipliersshape so the whole pricing/limits config is uniform.How
get_subscription_price_id(tier)now parsescopilot-tier-stripe-pricesand looks uptier.value— returnsNonewhen the flag is unset, non-dict, tier key missing, or value isn't a non-empty string.get_global_rate_limitsuses a new sibling_fetch_cost_limits_flag()helper (60s cache,cache_none=False) that extractsdaily/weeklyint keys independently and falls back to the existingChatConfigdefaults when any key is missing / non-int / negative. A brokendailydoesn't wipe outweekly(or vice versa).This PR removes 6 LD flags and introduces 2 replacements. To avoid a pricing/rate-limit outage, do this in LaunchDarkly first:
Create
copilot-tier-stripe-prices(type: JSON). Default variation = union of the currentstripe-price-id-*values:{ "PRO": "<current stripe-price-id-pro>", "MAX": "<current stripe-price-id-max>" }Omit BASIC / BUSINESS if those flags are unset today.
Create
copilot-cost-limits(type: JSON). Default variation = the current two flags' values:{ "daily": <current daily microdollars>, "weekly": <current weekly microdollars> }Merge this PR.
After deploy + smoke test, delete the six legacy flags:
stripe-price-id-{basic,pro,max,business}copilot-daily-cost-limit-microdollarscopilot-weekly-cost-limit-microdollarsTesting
pytest backend/copilot/rate_limit_test.py backend/data/credit_subscription_test.py backend/api/features/subscription_routes_test.py— rewritten to exercise the JSON flag shapes + fallback paths; passes locally.black --check/ruff check/isort --check— all clean.Checklist