Fix legacy backfill KG side effects

[Git-on-my-level]

Summary

• run canonical KG extraction for long-term legacy backfill rows after Firestore/vector/keyword side effects
• reconcile side effects when a deterministic backfill row already exists, so reruns repair missing KG/index state instead of silently skipping
• add kg_extraction_failures to the backfill report and regression tests for reviewed-long-term apply + rerun repair

Root cause

reviewed_long_term bucket backfill writes directly to canonical long_term items. That path synced Firestore, Pinecone, and Typesense, but never invoked the KG extraction hook used by short-term promotion. Worse, idempotent reruns returned early for already-present active rows, so a partially completed bucket could report present while leaving kg_extracted=false forever.

The read/projection gate observed during rollout is separate: dev backend is still in MEMORY_MODE=write, so default reads stay gated until rollout control advances.

Tests

• cd backend && pytest tests/unit/test_ws_c_backfill.py -q
• cd backend && pytest tests/unit/test_canonical_kg_promotion.py -q
• cd backend && pytest tests/unit/test_ws_b_short_term_lifecycle.py -q
• cd backend && python -m py_compile utils/memory/legacy_backfill.py tests/unit/test_ws_c_backfill.py
• pre-push fast checks passed, including selected backend unit tests and OpenAPI contract check

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eea8b0fd50

ℹ️ 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".

[chatgpt-codex-connector]

Preserve legacy updated_at after KG extraction

When applying a reviewed_long_term bucket, this call invokes set_canonical_memory_kg_extracted, which merges updated_at=datetime.now(...) onto the same memory item. That overwrites the legacy updated_at value that _apply_one_legacy_row just preserved for bucketed rows, so old reviewed memories become freshly updated immediately after KG extraction and can be misordered or look newly edited. Consider marking kg_extracted without touching the product memory timestamp, or restoring the legacy timestamp after KG succeeds.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reconcile KG before skipping completed resumes

This KG repair path only runs for rows that enter the bulk backfill_user loop. If a pre-fix bulk backfill already completed, the default resume=True path sets start_index to source_count, so rerunning scripts/backfill_legacy_memories.py --uid ... never reaches this extraction call and leaves existing long-term rows with missing KG side effects. The repair needs to run for completed checkpoints as well, or the operator has to know to pass --no-resume even though the report otherwise looks complete.

Useful? React with 👍 / 👎.

[cubic-dev-ai]

3 issues found across 2 files

Confidence score: 3/5

• In backend/utils/memory/legacy_backfill.py, _sync_backfill_side_effects can overwrite the preserved legacy timestamp by merging a fresh updated_at during extract_kg_for_promoted_memory, which risks silently losing historical ordering/metadata semantics after backfill — guard that merge path so canonical timestamps remain immutable for promoted legacy rows before merging.
• In backend/utils/memory/legacy_backfill.py, KG reconciliation is tied to _apply_one_legacy_row, so a resume=True run that skips the row loop may never execute reconciliation; this can leave data partially backfilled with stale/incomplete KG state — add a post-loop reconciliation step (or explicit resume-time invocation) before merge.
• tests/unit/test_canonical_memory_vectors.py::test_legacy_back... still unpacks _apply_one_legacy_row as a 5-tuple while the function now returns six values, so CI/test coverage for this path is currently unreliable and may fail or miss regressions — update the test contract to the new return shape 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/utils/memory/legacy_backfill.py">

<violation number="1" location="backend/utils/memory/legacy_backfill.py:453">
P2: The existing canonical vector regression test still unpacks `_apply_one_legacy_row` as a 5-tuple, while this signature now returns six values. That leaves `tests/unit/test_canonical_memory_vectors.py::test_legacy_backfill_idempotent_skip_resyncs_vector` failing with `ValueError: too many values to unpack`; updating that caller to capture or ignore the new KG failure flag would keep the test suite aligned with the API change.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The existing canonical vector regression test still unpacks _apply_one_legacy_row as a 5-tuple, while this signature now returns six values. That leaves tests/unit/test_canonical_memory_vectors.py::test_legacy_backfill_idempotent_skip_resyncs_vector failing with ValueError: too many values to unpack; updating that caller to capture or ignore the new KG failure flag would keep the test suite aligned with the API change.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/utils/memory/legacy_backfill.py, line 453:

<comment>The existing canonical vector regression test still unpacks `_apply_one_legacy_row` as a 5-tuple, while this signature now returns six values. That leaves `tests/unit/test_canonical_memory_vectors.py::test_legacy_backfill_idempotent_skip_resyncs_vector` failing with `ValueError: too many values to unpack`; updating that caller to capture or ignore the new KG failure flag would keep the test suite aligned with the API change.</comment>

<file context>
@@ -448,22 +450,27 @@ def _apply_one_legacy_row(
     db_client,
     bucket: Optional[LegacyBackfillBucket] = None,
-) -> tuple[MemoryControlState, bool, Optional[str], bool, bool]:
+) -> tuple[MemoryControlState, bool, Optional[str], bool, bool, bool]:
     """Write one canonical item. Returns control, write status, and side-effect status."""
     legacy_id = legacy_row.get("id") or f"legacy_{index}"
</file context>