Skip to content

fix(db): back-fill last_ping_at + last_pinged_reset_key on provider_connections - #12470

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
KooshaPari:fix/reasoning-routing-last-ping-at
Sep 5, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
KooshaPari:fix/reasoning-routing-last-ping-at

Conversation

@KooshaPari

Copy link
Copy Markdown
Contributor

Problem

Copilot AI lite review requested due to automatic review settings September 2, 2026 12:24

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 reached their quota limit.

diegosouzapw and others added 2 commits September 3, 2026 10:17
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@KooshaPari

Copy link
Copy Markdown
Contributor Author

Note (transparency)

I'm posting this on the PRs I opened during a self-imposed WAITING window. There's a pending handoff in my local state (~/.forge/handoffs/omniroute-handoff-WAITING-2026-09-03.md) that I'd intended to honor before opening additional PRs. The handoff flagged a contributor-graph concern I should have surfaced before broadening scope.

What I'm doing now:

  1. Not retracting any of these PRs — every one addresses an open issue, has tests/lint where applicable, and is independently useful. They stand on their merits.
  2. Continuing the upstream-PR campaign in parallel with the handoff, per operator direction.
  3. Surfacing the WAITING state here so maintainers have full context, not just the PR diff.

If any of these PRs shouldn't have been opened in your view, the comment-thread on each is the right place to flag it — I'll defer.

Refs: #12546 #12570 #12576 #12272 #12084 #11544 #12501 (the issues each one addresses).

— KooshaPari

@diegosouzapw
diegosouzapw merged commit 891cb26 into diegosouzapw:release/v3.8.51 Sep 5, 2026
15 of 16 checks passed
diegosouzapw added a commit that referenced this pull request Sep 5, 2026
…video turns (#12150 P2b) (#12707)

Merged, with one column-reconciliation gap closed.

The fail-closed reasoning is right and the comments carry it well: a stored snapshot whose cues were replaced by `[redacted-video-transcript]` must not be rehydrated as continuation history, because forwarding placeholder text upstream as if it were the client's real turn is worse than making the client resend. Treating it exactly like `previous_response_not_found` means no new client-visible behaviour to document. Migration 173 does not collide — the tip runs to 172.

**What I added:** `video_content_removed` to `ensureCallLogsColumns` in `src/lib/db/schemaColumns.ts`, plus a case in `tests/unit/db-schema-columns-split.test.ts`.

`resolvePreviousResponseState` now SELECTs that column on every `previous_response_id` lookup. Migration 173 creates it, but this repo carries a separate reconciliation path for lineages that skipped a migration — and on such a database the SELECT would throw `no such column: video_content_removed` instead of failing closed. That is the same hole #12470 closed for `provider_connections.last_ping_at` earlier today, so the pattern was fresh. Verified red-then-green: stubbing the new reconciliation out drops the suite to 8/9; restored, 9/9.

Validated on `release/v3.8.51`: `responses-continuation-store`, `save-call-log-persistence`, `video-bridge-log-redaction` and `db-schema-columns-split` all green (54 focused tests, 0 failures). `typecheck:core` and `lint` clean. The integration run logs `[DB] Added call_logs.video_content_removed column`, which is the reconciliation firing on a fresh test database.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…onnections (diegosouzapw#12470)

Merged. Clean, surgical fix with its own regression guard.

`ensureProviderConnectionsColumns()` reconciles the base columns that later data migrations assume, but `last_ping_at` / `last_pinged_reset_key` were only ever created by `123_quota_auto_ping` — so a lineage that skipped it kept a table that the quota auto-ping writes cannot target. Adding them to the reconciliation list is exactly the right place.

Validated on `release/v3.8.51`: `tests/unit/db-schema-columns-split.test.ts` 10/10, including your new `back-fills last_ping columns on a pre-123 lineage` case and the idempotency re-run. `typecheck:core` clean, `check-file-size` OK. The `changelog.d/fixes/` fragment was already correct.

Thank you — this is the shape a fix should have: root cause named, minimal diff, test that fails without it.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…video turns (diegosouzapw#12150 P2b) (diegosouzapw#12707)

Merged, with one column-reconciliation gap closed.

The fail-closed reasoning is right and the comments carry it well: a stored snapshot whose cues were replaced by `[redacted-video-transcript]` must not be rehydrated as continuation history, because forwarding placeholder text upstream as if it were the client's real turn is worse than making the client resend. Treating it exactly like `previous_response_not_found` means no new client-visible behaviour to document. Migration 173 does not collide — the tip runs to 172.

**What I added:** `video_content_removed` to `ensureCallLogsColumns` in `src/lib/db/schemaColumns.ts`, plus a case in `tests/unit/db-schema-columns-split.test.ts`.

`resolvePreviousResponseState` now SELECTs that column on every `previous_response_id` lookup. Migration 173 creates it, but this repo carries a separate reconciliation path for lineages that skipped a migration — and on such a database the SELECT would throw `no such column: video_content_removed` instead of failing closed. That is the same hole diegosouzapw#12470 closed for `provider_connections.last_ping_at` earlier today, so the pattern was fresh. Verified red-then-green: stubbing the new reconciliation out drops the suite to 8/9; restored, 9/9.

Validated on `release/v3.8.51`: `responses-continuation-store`, `save-call-log-persistence`, `video-bridge-log-redaction` and `db-schema-columns-split` all green (54 focused tests, 0 failures). `typecheck:core` and `lint` clean. The integration run logs `[DB] Added call_logs.video_content_removed column`, which is the reconciliation firing on a fresh test database.
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