Skip to content

Fix #582: include closed sessions in drift baselines - #725

Merged
anilmurty merged 1 commit into
Metabuilder-Labs:mainfrom
sanmaxdev:fix/582-terminal-drift-sessions
Aug 15, 2026
Merged

Fix #582: include closed sessions in drift baselines#725
anilmurty merged 1 commit into
Metabuilder-Labs:mainfrom
sanmaxdev:fix/582-terminal-drift-sessions

Conversation

@sanmaxdev

@sanmaxdev sanmaxdev commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Explicitly closed sessions were valid terminal history, but the DuckDB drift queries selected only sessions marked completed. Agents using the close endpoint could therefore miss valid history and fail to form a drift baseline.

Summary

  • use the shared TERMINAL_STATUSES set in both DuckDB drift-history queries
  • keep every status value parameterized with generated placeholders
  • add an end-to-end regression covering retrieval, counting, and baseline construction

Related issue

Closes #582

Behavior verified

The synthetic fixture contains one closed session and one completed session for the same agent.

  • before: 1 session counted, so the 2-session baseline threshold was not reached
  • after: 2 sessions counted and returned, producing a baseline with sessions_sampled == 2
  • measured eligibility shift: 1 to 2 sessions, a 100% increase for this fixture
  • measured baseline input average: 300 tokens for the pre-fix completed-only population versus 200 tokens after including both terminal statuses, a 33.3% decrease

Tests / Verification

  • new regression test fails against the pre-fix implementation with assert 1 == 2
  • pytest tests/synthetic/test_drift_detection.py::test_terminal_sessions_feed_drift_baseline -v passed
  • relevant drift and storage checks passed: 59 tests
  • pytest tests/unit/ tests/synthetic/ tests/agents/ tests/integration/ -q passed: 5,586 passed, 18 skipped, 2 xfailed
  • ruff check tokenjam/ clean
  • mypy tokenjam/ clean
  • git diff --check clean

What's NOT in this PR

  • no schema, API, CLI, or drift-threshold changes
  • no unrelated cleanup of pre-existing test-file lint findings

Checklist

  • Tests pass (pytest tests/unit/ tests/synthetic/ tests/agents/ tests/integration/)
  • Lint clean (ruff check tokenjam/)
  • Type check clean (mypy tokenjam/)
  • Contributor docs updated (not needed, no architecture change)
  • Test spans use tests/factories.py (not raw NormalizedSpan)

@anilmurty ready for review.

Explicitly closed sessions are terminal history, but the DuckDB drift
queries only selected sessions marked completed. This prevented valid
history from reaching baseline thresholds.

Use the shared terminal-status set for both storage reads and pin the
full baseline path with a closed-plus-completed regression fixture.

Closes Metabuilder-Labs#582

Co-Authored-By: anshss <noreply@github.com>
@sanmaxdev
sanmaxdev requested a review from anilmurty as a code owner August 15, 2026 14:35
@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes explicitly closed sessions eligible for drift-history retrieval and baseline thresholds alongside completed sessions.

  • Reuses the shared terminal-status set in both DuckDB history queries.
  • Generates parameterized placeholders for every terminal status.
  • Adds a regression test covering retrieval, counting, and two-session baseline construction.

Confidence Score: 5/5

The PR appears safe to merge, with the intended terminal-session behavior covered by a focused regression test.

The shared terminal-status tuple is bound safely and consistently in both retrieval and count queries, and the resulting baseline behavior matches the stated session-lifecycle contract.

Important Files Changed

Filename Overview
tokenjam/core/db.py Broadens both drift-history queries to closed and completed sessions using correctly matched parameterized placeholders.
tests/synthetic/test_drift_detection.py Adds end-to-end regression coverage proving closed and completed sessions jointly satisfy baseline construction.

Reviews (1): Last reviewed commit: "fix: include closed sessions in drift ba..." | Re-trigger Greptile

@anilmurty
anilmurty merged commit 58327c2 into Metabuilder-Labs:main Aug 15, 2026
7 checks passed
@anilmurty

Copy link
Copy Markdown
Contributor

Merged — thanks @sanmaxdev. Three things I want to call out specifically, because they're the parts that made this quick to review:

You consumed the existing TERMINAL_STATUSES instead of minting a second status set. That constant's docstring exists precisely to prevent a second copy, and this is its third consumer. You also stayed stricter than the precedent the issue pointed you at — plain concatenation for the placeholder list where status.py uses an f-string, which keeps Critical Rule 7 unambiguous at a glance.

The measured shift in the PR body is reproducible to the digit. I re-ran build_baseline against your fixture and got the count shift and the 300→200 average exactly as stated, and reverting only db.py reproduces the assert 1 == 2 you quoted. A regression test whose load-bearing-ness is claimed and true is worth more than it sounds — this repo has shipped decorative guards before.

It also fixes a second bug you didn't claim. Those two methods have six other callers. tj status was falling back to the newest completed session, so an agent whose most recent terminal session was closed displayed a stale session as its latest — while the Lens archive, already on TERMINAL_STATUSES, showed the correct one. Those two surfaces now agree.

One optional follow-up, not for this PR: both methods are still named get_completed_* while now returning all terminal sessions. Renaming touches the StorageBackend protocol, ApiBackend, the MCP shim and the parity classification, so it was right to leave it out of scope — but the stale docstring at core/drift.py:218 could ride along with it if you ever pick it up.

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.

Drift baselines silently exclude 'closed' sessions: get_completed_sessions filters on status='completed' only

2 participants