chore(ga): restore merge soft-close, drop dead refund path, dedup tests#34
Merged
Conversation
…up test fixture - service.py merge_workspaces(): replace the hard DELETE on colliding live edges with the bi-temporal soft-close used elsewhere in the codebase (Store._deduplicate_live_edges): merge weight/provenance/valid_from/ ingested_at and live edge_supports onto the surviving edge, then close the losing edge with valid_to+expired_at and a canonical_deduplicated_into provenance marker instead of deleting it. Ported from the validated closed-PR #30 (codex/commercial-v1-ga) implementation. Drops the now-orphaned _dumps import (its only call site was the removed hard-delete path). - tests/test_workspace_ops.py: update test_merge_deduplicates_colliding_live_edges to assert soft-close semantics (losing edge still present with valid_to/expired_at set and the canonical_deduplicated_into marker pointing at the survivor) instead of asserting the row is gone. - billing.py: remove polar_webhook()'s unreachable second refund/revoke dispatch (order.refunded / subscription.revoked / subscription.updated status==revoked) — the _REVOKING_EVENTS guard earlier in the function always returns first, so this code never ran. Also removes the now-fully-unused _revoke_refunded_order, _revoke_subscription_event, _polar_subscription_id and _polar_order_id (confirmed zero other callers/uses repo-wide via grep before deleting). - tests/test_billing.py: remove the duplicate def _registry_rows() (two byte-identical definitions; Python silently kept only the second). Validated: ruff check . clean; pytest tests/ 1605 passed, 14 skipped, 0 failed (identical to unmodified origin/main in this environment, confirming no regressions).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
Pre-PR Code Review — APPROVE ✅CI: 8/8 checks passing (all green) Analysis
Verdict: APPROVE — Clean consolidation PR. Dead code removed, soft-close restored, tests updated. CI green. Ready to merge pending branch protection approval from a human reviewer. |
…e_workspaces The soft-close support move (UPDATE edge_supports SET edge_id=...) was a plain UPDATE. idx_edge_support_live_unique is a partial unique index on (edge_id, memory_id, source_kind) WHERE live, so if the surviving target edge already had a live support for the same memory (same memory_id + source_kind), retargeting the source edge's identical live support onto it violated the index and raised sqlite3.IntegrityError, rolling back the whole merge transaction. Main's prior hard-delete code guarded against this with UPDATE OR IGNORE; the soft-close port dropped that safeguard. Fix: use UPDATE OR IGNORE for the move (matches main's original convention), then soft-close (valid_to+expired_at) whatever live support OR IGNORE left behind on the source edge, so nothing stays live on a now-closed edge -- mirroring how Store._deduplicate_live_edges() closes retired supports. Added test_merge_deduplicates_colliding_live_edges_with_colliding_support to tests/test_workspace_ops.py: both workspaces' colliding edges are backed by the same memory id, so the support move itself collides. Verified this test fails with the exact predicted IntegrityError against the pre-fix code and passes against the fix.
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.
Consolidated GA cleanup (follow-up to #24/#25/#30):
merge_workspaces()collision handling (ported from Codex/commercial v1 ga #30), replacing feat(commercial-v1-ga): sync working-tree changes for v1 GA #24's hardDELETE FROM edges. Matches the canonicalStore._deduplicate_live_edges()convention (valid_to + expired_at + canonical_deduplicated_into marker); preserves temporal history.polar_webhook()(2nd refund/revoke dispatch pre-empted by the_REVOKING_EVENTSguard) plus its now-unused helpers (grep-confirmed no other callers).test_merge_deduplicates_colliding_live_edgesto assert soft-close semantics; dedup the byte-identical_registry_rows()in test_billing.py (supersedes fix(tests): remove duplicate _registry_rows() definition in test_billing.py #32).Validated locally: ruff clean; pytest 1605 passed / 14 skipped / 0 failed (identical pass count to unmodified main in the same env).