fix(backend/copilot): actionable per-UUID reasons on memory forget failures - #13672
Conversation
…rget failures
memory_forget_confirm's soft delete used to report a bare "0 invalidated,
N failed" with no reason. The original root cause was that the shipped
_soft_delete_edges set timestamps with Cypher's no-arg datetime(), which
FalkorDB rejects ("Unknown function 'datetime'"); the raised error was
swallowed and every soft delete silently failed while hard delete (no
datetime()) worked. The dream-pass refactor already moved the timestamp to
a Python-bound $now param, fixing the functional failure — but the silent,
reasonless failure path remained.
_retract_edges and _hard_delete_edges now return per-UUID MemoryForgetFailure
records distinguishing a real query error (exception type + message) from a
plain no-match, and the confirm tool surfaces those reasons in both the
response's new `failures` field and its human/model-readable message.
Adds a regression guard pinning that the soft-delete Cypher never reintroduces
datetime().
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
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 (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (18)
WalkthroughMemory forget operations now return structured per-UUID failure reasons for unmatched edges and exceptions. Confirmation responses expose these failures, retain failed UUIDs, and include bounded failure details in human-readable messages. Tests cover deletion, retraction, timestamp binding, and propagation. ChangesMemory forget failure reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/copilot/tools/graphiti_forget.py`:
- Around line 49-58: The _delete_error_reason function must stop exposing raw
driver exception text through MemoryForgetFailure.reason. Sanitize the returned
message to include only the exception type and a safe actionable value such as
exc.args[0], while logging the full exception with exc_info=True in the
delete-error handling path for server-side debugging.
🪄 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: 90e512e1-3beb-41b4-92f9-ad6fbb51af2c
📒 Files selected for processing (3)
autogpt_platform/backend/backend/copilot/tools/graphiti_forget.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/models.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
- GitHub Check: type-check (3.13)
- GitHub Check: type-check (3.12)
- GitHub Check: end-to-end tests
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: lint
- GitHub Check: types
- GitHub Check: lint
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/graphiti_forget_test.py
🧠 Learnings (16)
📚 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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.py
📚 Learning: 2026-06-06T12:22:37.648Z
Learnt from: anvyle
Repo: Significant-Gravitas/AutoGPT PR: 13302
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:579-583
Timestamp: 2026-06-06T12:22:37.648Z
Learning: When writing LLM-facing instruction strings that trigger tool calls in this AutoGPT codebase, use the exact registered tool name `view_agent_output` (as defined in `backend/copilot/tools/agent_output.py` via its `name` property and exported via `TOOL_REGISTRY`). Do not reference the bare name `agent_output`, since it is not a valid tool name and will cause tool invocation to fail.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.py
📚 Learning: 2026-03-04T12:19:39.243Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12279
File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188
Timestamp: 2026-03-04T12:19:39.243Z
Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.py
📚 Learning: 2026-03-31T14:22:26.566Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12622
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:223-236
Timestamp: 2026-03-31T14:22:26.566Z
Learning: In files under autogpt_platform/backend/backend/copilot/tools/, ensure agent graph enrichment uses the typed Pydantic model `backend.data.graph.Graph` for `AgentInfo.graph` (i.e., `Graph | None`), not `dict[str, Any]`. When enriching with graph data (e.g., `_enrich_agents_with_graph`), prefer calling `graph_db().get_graph(graph_id, version=None, user_id=user_id)` directly to retrieve the typed `Graph` object rather than routing through JSON conversions like `get_agent_as_json()` / `graph_to_json()`.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.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/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.pyautogpt_platform/backend/backend/copilot/tools/graphiti_forget.py
🔇 Additional comments (3)
autogpt_platform/backend/backend/copilot/tools/models.py (1)
987-998: LGTM!Also applies to: 999-1005
autogpt_platform/backend/backend/copilot/tools/graphiti_forget.py (1)
270-352: LGTM! Failure propagation and control flow for_execute,_retract_edges, and_hard_delete_edgescorrectly distinguish no-match from query errors and thread the structuredMemoryForgetFailurelist through to the response, consistent with the confirmed test expectations.Also applies to: 510-571
autogpt_platform/backend/backend/copilot/tools/graphiti_forget_test.py (1)
7-16: LGTM! Solid coverage of no-match vs. query-error distinction, thedatetime()regression guard, and end-to-end propagation intoMemoryForgetConfirmResponse.Also applies to: 84-85, 110-222
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13672 +/- ##
==========================================
- Coverage 77.08% 77.07% -0.01%
==========================================
Files 2774 2774
Lines 210984 211099 +115
Branches 20170 20247 +77
==========================================
+ Hits 162629 162715 +86
- Misses 43892 43975 +83
+ Partials 4463 4409 -54
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Add a str-Enum MemoryForgetFailureCode (NO_MATCH, QUERY_ERROR) alongside the free-text reason on each MemoryForgetFailure so the frontend/model can branch on a stable code without parsing prose. Field is additive and backward-compatible. Regenerate openapi.json for the schema change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kcze
left a comment
kcze
left a comment
There was a problem hiding this comment.
🤖 Review done, no findings.
…1-bug-memory_forget_confirm-soft-delete-silently-fails
…etail Address PR review: - _delete_error_reason now surfaces only the exception type + first arg, never the full repr(exc), so raw driver text (potential connection/host details) can't leak into the model/user-facing reason. Full exception still logged server-side with exc_info=True. - _build_confirm_message caps inlined failure reasons at _MAX_FAILURE_DETAIL and appends "…and N more", so a wholesale-failure batch can't push the tool output past its size threshold and lose all detail; the full per-UUID list remains in the structured `failures` field. - Add tests for a mixed success/failure batch (two-part message + co-populated deleted_uuids and failures) and for the message cap / no-failure early return. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1-bug-memory_forget_confirm-soft-delete-silently-fails
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
🔍 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.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 2 conflict(s), 0 medium risk, 4 low risk (out of 6 PRs with file overlap) Auto-generated on push. Ignores: |
|
/review |
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
2 participants
Why / What / How
Why:
memory_forget_confirmcalled with the defaulthard_delete: falsefails silently — it returns0 invalidated, N failedwith no reason, while the same UUIDs succeed immediately withhard_delete: true(SECRT-2371).Root cause (confirmed from git history): the originally shipped
_soft_delete_edgesset its timestamps with Cypher's no-argdatetime():FalkorDB does not implement
datetime()— it raisesUnknown function 'datetime'. That exception was caught by a bareexcept Exceptionand the UUID was silently pushed onto thefailedlist. Hard delete never calleddatetime(), so it worked every time. The "recently-created edges" pattern noted in the ticket is spurious correlation — the shipped soft delete failed for all edges; the reporter only ever tested recent ones.The dream-pass refactor (#13243) already migrated the soft-delete path to a Python-generated
$nowparameter (_retract_edges/_now_iso()), which fixes the functional failure ondev. What remained — and is explicitly called out as a bug in its own right in the ticket — is the silent, reasonless failure path: even a legitimate no-match returned a bare count with nothing for the model to act on.What: Every forget failure now carries an actionable, per-UUID reason with a stable machine-switchable code, surfaced both structurally (response schema) and in the model-visible message. Also adds a regression guard so the
datetime()root cause cannot be reintroduced.How — error surface:
MemoryForgetFailuremodel with three fields:uuid,code(enum), andreason(free text).MemoryForgetFailureCodestr-Enum with stable valuesno_matchandquery_error, so the frontend/model can branch on the code without parsing prose (consumers must tolerate unknown future codes).failures: list[MemoryForgetFailure]field onMemoryForgetConfirmResponse(additive, backward-compatible; the pre-existingfailed_uuidslist is retained)._retract_edgesand_hard_delete_edges(both only called by the confirm path) now distinguish a real query error (query_error, carrying the exception type + message, e.g.Unknown function 'datetime') from a plain no-match (no_match), returning(succeeded, failures)._build_confirm_messagespells out each failed UUID and its reason instead of a bare "N failed".frontend/src/app/api/openapi.jsonfor the new response schema.Decisions (confirmed with maintainer):
hard_deletedefault staysfalse(soft delete). Defaulting to irreversible removal would be a dangerous default.dev(already fixed); this PR only adds a regression guard test pinning that the soft-delete Cypher never reintroducesdatetime().Scope kept minimal:
_soft_delete_edges,mark_edges_superseded, andinvalidate_entity_direct_neighbors(used by the dream pass) are untouched — only the two confirm-path helpers changed.Fixes SECRT-2371
Changes 🏗️
MemoryForgetFailureCode(str-Enum:no_match,query_error) andMemoryForgetFailure(uuid,code,reason); addfailurestoMemoryForgetConfirmResponse(models.py).(code, reason)failures — query-error vs. no-match — from_retract_edgesand_hard_delete_edges(graphiti_forget.py)._build_confirm_message.openapi.json.datetime()regression guard.Checklist 📋
For code changes:
poetry run pytest backend/copilot/tools/graphiti_forget_test.py— 20 passed (mocked driver, no infra)poetry run formatandpoetry run lint— cleanopenapi.jsonFor configuration changes:
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changes