feat(memory): add first-user dev read proof tooling#9025
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c77d5a7e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "reviewed": data.get("reviewed") if isinstance(data.get("reviewed"), bool) else True, | ||
| "user_review": data.get("user_review"), |
There was a problem hiding this comment.
When the source memory_items row has canonical product metadata in promotion.user_review (the shape used by MemoryItem and filtered by filter_canonical_default_visible_items), this code reads only the top-level user_review and _require_safe_active_item never rejects promotion.user_review is False. In that case an item the user explicitly rejected can still be written into the compatibility projection with its content and later returned by projection-backed /v3 reads, so the projection builder needs to honor the canonical promotion fields or skip rejected rows.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,255 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
The backend guide says new test files must be added to backend/test.sh; I checked the hardcoded fallback list in that script and tests/unit/test_first_user_memory_tools.py is not referenced. As a result, a default local bash backend/test.sh run will skip the only tests covering these new projection/proof tools, making regressions easy to miss unless someone runs this file explicitly.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 8 files
Confidence score: 3/5
- In
backend/scripts/apply_first_user_v3_projection.py,_projection_itemcan stampcreated_atfromupdated_at, which can misorder memories and distort timeline-dependent behavior if merged as-is — switch the timestamp fallback socreated_atis sourced from creation/capture fields first. - In
backend/scripts/apply_first_user_v3_projection.py,_require_safe_active_itemappears to omit user-rejection checks, so explicitly rejected memories could still enter the projection and surface to users — add auser_review/rejection gate before including items. - In
backend/scripts/first_user_memory_e2e_proof.py, non-list/v3/memoriesresponses are coerced to[]and missingprojection_generationcan be treated as matching, so the proof can report success while projection state is broken — fail fast on non-list responses and requireprojection_generationto be present for fence validation. backend/test.shdoes not include the new projection/E2E test file, so these regressions may not be caught in default runs even after fixes — add the new test path to the hardcoded test list before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/scripts/apply_first_user_v3_projection.py">
<violation number="1" location="backend/scripts/apply_first_user_v3_projection.py:9">
P2: This new test file isn't included in `backend/test.sh`'s hardcoded test file list. A default `bash backend/test.sh` run will skip these tests, meaning regressions in the projection and E2E proof tools won't be caught unless someone explicitly targets this file. Adding it to `test.sh` ensures consistent coverage in local and CI workflows.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,375 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
P2: This new test file isn't included in backend/test.sh's hardcoded test file list. A default bash backend/test.sh run will skip these tests, meaning regressions in the projection and E2E proof tools won't be caught unless someone explicitly targets this file. Adding it to test.sh ensures consistent coverage in local and CI workflows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/scripts/apply_first_user_v3_projection.py, line 9:
<comment>This new test file isn't included in `backend/test.sh`'s hardcoded test file list. A default `bash backend/test.sh` run will skip these tests, meaning regressions in the projection and E2E proof tools won't be caught unless someone explicitly targets this file. Adding it to `test.sh` ensures consistent coverage in local and CI workflows.</comment>
<file context>
@@ -0,0 +1,375 @@
+state/items paths for the requested user.
+"""
+
+from __future__ import annotations
+
+import argparse
</file context>
1 participant
Summary
/v3/memorieschecksTests
cd backend && python3 -m pytest tests/unit/test_first_user_memory_tools.py tests/unit/test_backend_runtime_env_validator.py tests/unit/test_v3_production_runtime_wiring.py -qcd backend && python3 scripts/validate-backend-runtime-env.py --env devcd backend && python3 scripts/validate-backend-runtime-env.py --env prodpython3is 3.11.15 vs.python-version3.11.14, several backend packages are not importable, and OpenAPI export failed to start). The branch was pushed with--no-verifyafter the targeted tests and runtime-env validators above passed.Safety
vi7SA9ckQCe4ccobWNxlbdcNdC23