Skip to content

Remove PartialModes module and consolidate into SnapshotQuery#4344

Merged
alco merged 4 commits into
mainfrom
alco/partial-modes-dead-code
May 19, 2026
Merged

Remove PartialModes module and consolidate into SnapshotQuery#4344
alco merged 4 commits into
mainfrom
alco/partial-modes-dead-code

Conversation

@alco
Copy link
Copy Markdown
Member

@alco alco commented May 18, 2026

Summary

  • Remove dead code: PartialModes.query_move_in/3 and query_move_in_async/3 that had no callers since a04b259
  • Move query_subset/2 from PartialModes into SnapshotQuery where it logically belongs alongside execute_for_shape/2
  • Delete the now-empty PartialModes module entirely

Test plan

  • Existing tests pass (no behavior change, only code reorganization)
  • CI green

🤖 Generated with Claude Code

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

❌ 17 Tests Failed:

Tests completed Failed Passed Skipped
3711 17 3694 52
View the top 3 failed test(s) by shortest run time
test/runtime-dsl.test.ts > N: wake primitives verification > N5: runFinished wake records the finished child on the parent stream
Stack Traces | 15.1s run time
Error: Timeout (15000ms) waiting for entity history on /wake-summary-parent-n4/wake-summary-1
[
  {
    "args": {},
    "entityType": "wake-summary-parent-n4",
    "operation": "insert",
    "type": "entity_created"
  },
  {
    "from": "/principal/system%3Aruntime-dsl-test",
    "operation": "insert",
    "payload": "spawn trio",
    "type": "inbox"
  },
  {
    "key": "run-0",
    "operation": "insert",
    "status": "started",
    "type": "run"
  },
  {
    "key": "step-0",
    "operation": "insert",
    "status": "started",
    "stepNumber": 1,
    "type": "step"
  },
  {
    "key": "msg-0",
    "operation": "insert",
    "status": "streaming",
    "type": "text"
  },
  {
    "delta": "spawned:3",
    "key": "msg-0:0",
    "operation": "insert",
    "runId": "run-0",
    "textId": "msg-0",
    "type": "text_delta"
  },
  {
    "key": "msg-0",
    "operation": "update",
    "status": "completed",
    "type": "text"
  },
  {
    "finishReason": "stop",
    "key": "step-0",
    "operation": "update",
    "status": "completed",
    "stepNumber": 1,
    "type": "step"
  },
  {
    "finishReason": "stop",
    "key": "run-0",
    "operation": "update",
    "status": "completed",
    "type": "run"
  },
  {
    "key": "child:wake-summary-child-n4:wake-summary-1-alpha",
    "manifest": {
      "entityType": "wake-summary-child-n4",
      "entityUrl": "/wake-summary-child-n4/wake-summary-1-alpha",
      "id": "wake-summary-1-alpha",
      "key": "child:wake-summary-child-n4:wake-summary-1-alpha",
      "kind": "child",
      "wake": "runFinished"
    },
    "operation": "insert",
    "type": "manifest"
  },
  {
    "key": "child:wake-summary-child-n4:wake-summary-1-bravo",
    "manifest": {
      "entityType": "wake-summary-child-n4",
      "entityUrl": "/wake-summary-child-n4/wake-summary-1-bravo",
      "id": "wake-summary-1-bravo",
      "key": "child:wake-summary-child-n4:wake-summary-1-bravo",
      "kind": "child",
      "wake": "runFinished"
    },
    "operation": "insert",
    "type": "manifest"
  },
  {
    "key": "child:wake-summary-child-n4:wake-summary-1-charlie",
    "manifest": {
      "entityType": "wake-summary-child-n4",
      "entityUrl": "/wake-summary-child-n4/wake-summary-1-charlie",
      "id": "wake-summary-1-charlie",
      "key": "child:wake-summary-child-n4:wake-summary-1-charlie",
      "kind": "child",
      "wake": "runFinished"
    },
    "operation": "insert",
    "type": "manifest"
  }
]
 ❯ waitForHistory test/runtime-dsl.ts:664:11
 ❯ test/runtime-dsl.test.ts:6261:27
test/runtime-dsl.test.ts > K: wiki coordination > K4: create_wiki rejects switching the topic on an existing wiki
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5546:3
test/runtime-dsl.test.ts > M: deep researcher coordination > M3: separate researcher entities keep child results isolated across later wakes
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:4968:3
test/runtime-dsl.test.ts > K: wiki coordination > K9: idempotent wiki recreation does not duplicate shared article rows
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5760:3
test/runtime-dsl.test.ts > J: debate coordination > J3: debate with only one durable side stays partial until the missing side arrives
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5328:3
test/runtime-dsl.test.ts > K: wiki coordination > K6: repeating create_wiki with the same topic and subtopics is idempotent
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5630:3
test/runtime-dsl.test.ts > K: wiki coordination > K10: same-topic wiki expansion adds only the missing article and updates later query coverage
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5791:3
test/runtime-dsl.test.ts > I: peer review coordination > I4: peer review with two configured reviewers summarizes only those durable rows
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5181:3
test/runtime-dsl.test.ts > I: peer review coordination > I3: peer review with one configured reviewer summarizes only that durable row
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5143:3
test/runtime-dsl.test.ts > K: wiki coordination > K8: wiki keeps durable child metadata and shared articles carry topic and author details
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5688:3
test/runtime-dsl.test.ts > I: peer review coordination > I1: peer review aggregates three reviewer writes through shared state
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5049:3
test/runtime-dsl.test.ts > K: wiki coordination > K3: get_wiki_status reports complete coverage after specialist articles land
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5521:3
test/runtime-dsl.test.ts > K: wiki coordination > K2: repeating create_wiki reuses existing specialists and only spawns missing subtopics
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5434:3
test/runtime-dsl.test.ts > M: deep researcher coordination > M1: researcher workers start from spawn initialMessage without an extra send
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:4915:3
test/runtime-dsl.test.ts > J: debate coordination > J1: debate parent reads both sides from shared state before issuing a ruling
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:5221:3
test/runtime-dsl.test.ts > D: shared state > D9: a setup-registered shared-state effect fires on the first wake write and survives a later wake
Stack Traces | 30s run time
Error: Test timed out in 30000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ test/runtime-dsl.test.ts:3396:3
test/runtime-dsl.test.ts > K: wiki coordination > K1: wiki specialists accumulate shared articles that a later query can read
Stack Traces | 30.1s run time
Error: Timeout (30000ms) waiting for shared:wiki_article x2 on shared state wiki-wiki-1
[]
 ❯ waitForHistory test/runtime-dsl.ts:664:11
 ❯ test/runtime-dsl.test.ts:5391:5

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@claude
Copy link
Copy Markdown

claude Bot commented May 18, 2026

Claude Code Review

Summary

Pure refactor: removes the dead query_move_in/3 and query_move_in_async/3 functions from Electric.Shapes.PartialModes (orphaned since a04b259), relocates the remaining query_subset/4 into Electric.Postgres.SnapshotQuery as execute_for_subset/4, and deletes the now-empty PartialModes module. Net -95 lines, no behavior change.

What's Working Well

  • Sensible consolidation — the moved function only ever delegated into SnapshotQuery.execute_for_shape/4, so co-locating it next to its only caller is the right call.
  • Naming alignment: execute_for_subset parallels the existing execute_for_shape, which reads better than the previous PartialModes.query_subset (which collided name-wise with Querying.query_subset).
  • Changeset present (.changeset/little-pigs-complain.md), correctly scoped as patch for @core/sync-service.
  • Public API surface is preserved via defdelegate query_subset/4 in Electric.Shapes, so the existing call site at lib/electric/shapes/api.ex:579 is untouched.
  • The live query_move_in_async implementation lives in consumer/effects.ex (the one with callers and tests); removing the orphan eliminates confusion about which copy is authoritative.
  • Both prior review suggestions were picked up cleanly in 221a8b7.

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

None.

Suggestions (Nice to Have)

None remaining. A grep across packages/sync-service and integration-tests confirms no lingering PartialModes references.

Issue Conformance

No linked issue, which is reasonable for a small cleanup PR. The PR description accurately describes the three mechanical changes (remove dead code, move query_subset, delete module). The "Test plan" checklist items in the PR body are still unchecked — worth confirming mix test was run locally, given there's no test churn but the module path moved.

Previous Review Status

Both suggestions from iteration 1 were addressed in 221a8b7:

  • Stale comment in integration-tests/tests/otel-export.lux:119 — updated to point at SnapshotQuery.execute_for_subset instead of the deleted PartialModes path.
  • Redundant opts[:stack_id] lookup in execute_for_subset — replaced with the already-bound stack_id variable at snapshot_query.ex:150.

No new issues introduced. LGTM.


Review iteration: 2 | 2026-05-18

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@alco alco self-assigned this May 18, 2026
Copy link
Copy Markdown
Contributor

@robacourt robacourt left a comment

Choose a reason for hiding this comment

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

🧹

@alco alco merged commit 4590862 into main May 19, 2026
160 of 165 checks passed
@alco alco deleted the alco/partial-modes-dead-code branch May 19, 2026 11:22
@github-actions
Copy link
Copy Markdown
Contributor

This PR has been released! 🚀

The following packages include changes from this PR:

  • @core/sync-service@1.6.6

Thanks for contributing to Electric!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants