Skip to content

Harden developer memories list against malformed records#7784

Open
tianmind-studio wants to merge 4 commits into
BasedHardware:mainfrom
tianmind-studio:codex/dev-memories-pagination-resilience
Open

Harden developer memories list against malformed records#7784
tianmind-studio wants to merge 4 commits into
BasedHardware:mainfrom
tianmind-studio:codex/dev-memories-pagination-resilience

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make CleanerMemory tolerate legacy Developer API memory records with invalid optional fields by coercing safe defaults.
  • Reject missing/empty memory IDs at model validation time, while keeping the list endpoint's per-record guard that skips id-less records before response serialization.
  • Convert numeric created_at / updated_at values into UTC datetimes instead of returning raw numbers.
  • Add HTTP coverage for paginated /v1/dev/user/memories responses containing malformed legacy records so a single bad record does not turn the page into a 500.

Regression check

With the new tests but backend/routers/developer.py temporarily restored to origin/main:

  • python -m pytest tests\unit\test_dev_api_folder_filters.py -q -> 2 failed, 16 passed, 2 warnings
  • failures covered a 500 response from /v1/dev/user/memories?limit=3&offset=7 and direct CleanerMemory(...) validation errors for legacy values

Testing

  • python -m pytest tests\unit\test_dev_api_folder_filters.py -q -> 19 passed, 2 warnings
  • python -m black --line-length 120 --skip-string-normalization routers\developer.py tests\unit\test_dev_api_folder_filters.py --check
  • python -m py_compile routers\developer.py tests\unit\test_dev_api_folder_filters.py
  • git diff --check

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the GET /v1/dev/user/memories Developer API endpoint so that a single malformed or legacy Firestore record no longer turns an entire paginated response into a 500. It adds per-field field_validator coercions to CleanerMemory (covering None ids, invalid categories, bad datetimes, non-list tags, stringified booleans, etc.) and a pre-filter that skips non-dict or id-less records before model_validate is called.

  • CleanerMemory now tolerates every previously-required field being absent or of the wrong type by coercing safe defaults, mirroring the hardening already present on the /v3/memories path.
  • A pre-filter guard (isinstance(memory, dict) and memory.get('id')) plus a try/except ValidationError loop replace the previous direct append, preventing any single bad record from bubbling up as an unhandled exception.
  • New HTTP-layer and unit tests in TestDevApiMemoriesHttpLayer cover a mixed-quality page (good record, legacy malformed record, locked record with null content, id-less record) and directly exercise each validator's edge values.

Confidence Score: 4/5

Safe to merge; the changes are additive hardening on a read path and the new validators are well-tested.

The coercion logic is thorough and the tests confirm the key failure scenarios. Two minor quality gaps exist: the coerce_datetime numeric branch silently delegates to Pydantic's internal timestamp coercion rather than explicitly calling datetime.fromtimestamp, and coerce_id allows an empty-string id to propagate through CleanerMemory when the model is used directly (e.g., as the PATCH response model) without the endpoint's pre-filter guarding it.

backend/routers/developer.py — specifically the coerce_id and coerce_datetime validators.

Important Files Changed

Filename Overview
backend/routers/developer.py Adds field_validator coercions to CleanerMemory so legacy/malformed Firestore records survive serialization; adds a pre-filter that skips non-dict or id-less records before model_validate; fixes a potential None[:70] crash in the locked-memory content truncation path.
backend/tests/unit/test_dev_api_folder_filters.py Adds TestDevApiMemoriesHttpLayer with an HTTP-layer test that exercises a mixed-quality page (good record, legacy malformed record, locked record with null content, id-less record) and a direct unit test for CleanerMemory validator edge values; both tests are clean and well-structured.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /v1/dev/user/memories] --> B[memories_db.get_memories]
    B --> C{For each record}
    C --> D{is dict AND has id?}
    D -- No --> E[Skip with warning]
    D -- Yes --> F{is_locked?}
    F -- Yes --> G[Truncate content to 70 chars]
    G --> H[CleanerMemory.model_validate]
    F -- No --> H
    H --> I{ValidationError?}
    I -- No --> J[Append to valid_memories]
    I -- Yes --> K[Skip with warning]
    J --> L[Return valid_memories]
Loading

Reviews (1): Last reviewed commit: "Make dev memories list tolerate malforme..." | Re-trigger Greptile

Comment thread backend/routers/developer.py Outdated
Comment thread backend/routers/developer.py Outdated

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardening on top of #7678 (already merged) — approve only since the 500 symptom is already fixed on main

@tianmind-studio tianmind-studio force-pushed the codex/dev-memories-pagination-resilience branch from d3995fd to 76fd13d Compare June 12, 2026 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants