Skip to content

test(runtime): drain observers around chat SQL budget capture - #1145

Closed
loveRhythm1990 wants to merge 3 commits into
matrixorigin:mainfrom
loveRhythm1990:fix/chat-sql-budget-observer-drain-20261027
Closed

loveRhythm1990 wants to merge 3 commits into
matrixorigin:mainfrom
loveRhythm1990:fix/chat-sql-budget-observer-drain-20261027

Conversation

@loveRhythm1990

@loveRhythm1990 loveRhythm1990 commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Capability assessment: N/A: test measurement boundaries only; no production capability, auth policy, or deployment behavior changes.

Summary

PR #1139's online integration lane fails ordinary_chat_and_readonly_rounds_have_bounded_total_sql: the warm-context scenario records 22 statements against the existing limit of 20. The warm-up response can finish before its detached observer, allowing that observer's authorization query into the next turn's capture. drain_background_runs does not own that observer. A SQLx connection-initialization statement also appears in the failed capture.

This independent fix prepares a bounded connection reserve before measurement, consumes the full warm-up SSE response and waits for its observer before starting capture, and drains the final measured run and waits for its observer before stopping capture. Shutdown drain is deliberately not called after warm-up because it closes admission for the measured turn. The probe uses the production observer's completion metrics and rejects errors, dropped work, extra completions, and missing completions. It retains the 20-statement raw total limit, transaction/control accounting, and the no-extra-SQL assertion for readonly tool rounds. No query inside the capture window is filtered out. The readonly-round delta also accounts for at most one exact account/session authentication snapshot when the existing 30-second cache expires. This does not match Memoria authority queries; duplicate authentication snapshots still fail, and every query still counts toward the raw limit.

Related issue

Follow-up validation fix for #1139 / #1138. This PR targets main independently; it does not replace or include the observer authorization change in #1139. After this lands, #1139 needs to incorporate the updated main and rerun CI.

Change type

  • Bug fix
  • Test
  • Documentation

User and compatibility impact

None. No runtime, authorization, schema, API, or deployment change. The probe remains a scripted-provider integration test against an isolated MatrixOne database.

Architecture and complexity delta

  • Canonical owner changed or extended: only the existing chat_sql_budget integration probe and its testing guide.
  • Existing implementations and callers searched: production observer dispatch/completion metrics, runtime drain, HTTP fixture bootstrap, canonical Memoria authority resolver, SQL capture and pool construction.
  • Superseded code, states, tables, shims, or self-only tests removed: replaced the incomplete post-response drain in this probe with run plus observer completion; no production state added.
  • If parallel implementations remain, their boundary and retirement condition: none; real production observer and authorization remain in use.

Verification

  • Passed: cargo fmt --all -- --check, git diff --check, and python3 scripts/ci/validate_repository.py (repository metadata check before the final small assertion/documentation follow-up).
  • Final patch compiled when applied to fix(memory): enforce per-user authority for turn observation #1139 head 27387141: cargo test -p astra-runtime --features e2e-hooks --test chat_sql_budget --no-run.
  • Offline target invocation passed: 1 passed, 5 database tests ignored, including the strict production-observer HTTP stub. This is not a claim that the database tests passed in that invocation.
  • Unmodified fix(memory): enforce per-user authority for turn observation #1139 reproduced a measurement failure against local MatrixOne: ordinary cold chat counted 16 statements, while three readonly discovery rounds counted 15 and failed the exact-delta assertion. The original CI failure was the distinct warm-context 22/20 total assertion (failed job).
  • Focused real-database validation on fix(memory): enforce per-user authority for turn observation #1139 plus the observer/pool-boundary patch passed both warm-context scenarios: ordinary chat 18 SQL, three readonly agent-list rounds 18 SQL, no control polls. For this local run only, the disposable validation copy selected those two scenarios; the committed probe retains all nine.
  • A preceding full local rerun produced five cold-scenario captures before it was stopped during slow fixture setup. One capture contained a genuine authentication-cache miss. Re-evaluating all five captures plus the two warm captures under the final exact-authentication allowance satisfies both the unchanged 20-statement cap and the readonly delta. Replaying the original missing-observer capture still fails, demonstrating that the allowance does not hide that regression.
  • Limit: the final full nine-scenario database test has not completed locally. The full GitHub CI run for the final commit remains pending; this PR stays draft. Full workspace make check was not completed.
  • Public entrypoint exercised: scripted /chat/stream, ordinary/warm-context chat and readonly discovery/agent-list. No live LLM is involved. No schema or production query changes; all database validation used a dedicated local test database, without QA/production mutations.

Self-review covered both modified files (2 reviewed, 0 skipped; Rust rules plus manual documentation review). The premature warm-up shutdown found during local testing was corrected before the successful focused warm run.

Final checklist

  • Updated the existing failing integration test at its owning layer.
  • Documented measurement boundaries and the focused invocation.
  • Checked the diff for sensitive information and generated files.
  • Conventional Commit title.

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.

1 participant