Skip to content

fix(python): retain memory event filters across reconnects - #1145

Merged
santoshkumarradha merged 1 commit into
Agent-Field:mainfrom
remote-controlled-man:codex/prflow-914d2c358e56
Nov 2, 2026
Merged

santoshkumarradha merged 1 commit into
Agent-Field:mainfrom
remote-controlled-man:codex/prflow-914d2c358e56

Conversation

@remote-controlled-man

@remote-controlled-man remote-controlled-man commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

MemoryEventClient stopped sending patterns, scope, and scope_id after a dropped connection or an initial failure followed by subscribe(). Snapshot the explicit connection parameters and reuse them for background connections, while preserving explicit connect() defaults.

Rebased onto current main after #1142 landed. Subscription-initiated connections retain the filters and use the shutdown fix's tracked background tasks, so closing the client can cancel them. Both sets of memory-event regressions pass together.

Prepared with AI assistance, independently reviewed and tested.

Type of change

  • Bug fix

Test plan

  • Real local WebSocket regressions for disconnects, initial connection failure, caller list mutation, connected no-op calls, and explicit unfiltered connections; shutdown and reconnect tests run together.
  • Python SDK Ruff and mypy checks.
  • ./scripts/test-all.sh (control plane, Go SDK, Python SDK with unchanged default coverage/markers, TypeScript SDK, and web UI). Go uses an isolated Linux Go 1.25.14 runtime with controlled public/invalid DNS; Python 3.12 and JavaScript run natively on macOS.
  • Memory event tests with websockets 12.0 and 15.0.1.
  • Python benchmark from .github/workflows/memory-metrics.yml.
  • make fmt tidy completed. Unrelated baseline formatter changes were restored; the two PR files already match the formatter and module files were unchanged.

Test coverage

  • No coverage baselines were lowered.
  • New code paths are covered by tests.
  • CI on the rebased head is pending completion.

Checklist

  • Read docs/CONTRIBUTING.md and docs/DEVELOPMENT.md.
  • Commits are signed and follow conventional-commits style. (Conventional-commits style; unsigned.)

Related issues / PRs

Source-discovered Python bug, separate from the TypeScript filter feature in #838. This branch now includes the Python shutdown fix merged in #1142.

Copilot AI balanced review requested due to automatic review settings November 1, 2026 04:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Performance

SDK Memory Δ Latency Δ Tests Status
Python 9.1 KB - 0.32 µs -9% ✓ ✓

✓ No regressions detected

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 88.20% 87.40% ↑ +0.80 pp 🟡
sdk-go 93.70% 92.00% ↑ +1.70 pp 🟢
sdk-python 94.73% 93.73% ↑ +1.00 pp 🟢
sdk-typescript 92.27% 90.42% ↑ +1.85 pp 🟢
web-ui 84.89% 84.79% ↑ +0.10 pp 🟡
aggregate 86.11% 85.75% ↑ +0.36 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 0 — ➖ no changes
sdk-go 0 — ➖ no changes
sdk-python 0 — ➖ no changes
sdk-typescript 0 — ➖ no changes
web-ui 0 — ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the filter persistence change and its reconnect coverage, including encoded parameters and initial failures. Focused memory-event tests pass locally.

@santoshkumarradha
santoshkumarradha added this pull request to the merge queue Nov 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 1, 2026
@santoshkumarradha

Copy link
Copy Markdown
Member

This is approved on its own merits. The protected merge queue currently marks it unmergeable because it overlaps #1142, which is ahead of it in the queue and changes the same memory-events lifecycle code. Please rebase it on current main after #1142 lands, then the checks can rerun with both fixes together.

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rebased diff is focused on preserving explicit memory-event filters through reconnects. The reconnect, failed-connect, copied-parameter, and unfiltered paths are covered; focused tests pass locally, and CLA plus required checks are green.

@santoshkumarradha
santoshkumarradha added this pull request to the merge queue Nov 2, 2026
Merged via the queue into Agent-Field:main with commit 232088a Nov 2, 2026
28 checks passed
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.

3 participants