-
Notifications
You must be signed in to change notification settings - Fork 96
fix: recover stale context limits after model switches #341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 5 commits
fdc66ff
7eff875
ce09be2
2d610d5
f4f6b55
6a5e30d
af22bd4
8549d11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /// <reference types="bun-types" /> | ||
|
|
||
| import { describe, expect, test } from "bun:test"; | ||
| import { Database } from "../../shared/sqlite"; | ||
| import { closeQuietly } from "../../shared/sqlite-helpers"; | ||
| import { LATEST_MIGRATION_VERSION, runMigrations } from "./migrations"; | ||
| import { initializeDatabase, LATEST_SUPPORTED_VERSION } from "./storage-db"; | ||
|
|
||
| function seedAppliedVersion(db: Database, version: number): void { | ||
| db.exec(` | ||
| CREATE TABLE schema_migrations ( | ||
| version INTEGER PRIMARY KEY, | ||
| description TEXT NOT NULL, | ||
| applied_at INTEGER NOT NULL | ||
| ); | ||
| `); | ||
| const insert = db.prepare( | ||
| "INSERT INTO schema_migrations (version, description, applied_at) VALUES (?, ?, ?)", | ||
| ); | ||
| for (let current = 1; current <= version; current += 1) { | ||
| insert.run(current, `seed v${current}`, Date.now()); | ||
| } | ||
| } | ||
|
|
||
| function columnNames(db: Database, table: string): string[] { | ||
| return (db.prepare(`PRAGMA table_info(${table})`).all() as Array<{ name: string }>).map( | ||
| (column) => column.name, | ||
| ); | ||
| } | ||
|
|
||
| describe("migration v80: tokenless usage observation timestamp", () => { | ||
| test("fresh databases include the timestamp and align the schema fence", () => { | ||
| const db = new Database(":memory:"); | ||
| try { | ||
| initializeDatabase(db); | ||
| runMigrations(db); | ||
|
|
||
| expect(columnNames(db, "session_meta")).toContain("last_usage_observed_at"); | ||
| expect(LATEST_SUPPORTED_VERSION).toBe(80); | ||
| expect(LATEST_SUPPORTED_VERSION).toBe(LATEST_MIGRATION_VERSION); | ||
| } finally { | ||
| closeQuietly(db); | ||
| } | ||
| }); | ||
|
|
||
| test("replaying from v79 preserves the observation time for legacy token usage", () => { | ||
| const db = new Database(":memory:"); | ||
| try { | ||
| seedAppliedVersion(db, 79); | ||
| db.exec(` | ||
| CREATE TABLE session_meta ( | ||
| session_id TEXT PRIMARY KEY, | ||
| last_context_percentage REAL DEFAULT 0, | ||
| last_input_tokens INTEGER DEFAULT 0, | ||
| last_response_time INTEGER | ||
| ); | ||
| INSERT INTO session_meta ( | ||
| session_id, last_context_percentage, last_input_tokens, last_response_time | ||
| ) VALUES ('ses-legacy', 50, 50000, 123); | ||
| `); | ||
|
|
||
| runMigrations(db); | ||
| runMigrations(db); | ||
|
|
||
| expect( | ||
| db | ||
| .prepare("SELECT last_usage_observed_at FROM session_meta WHERE session_id = ?") | ||
| .get("ses-legacy"), | ||
| ).toEqual({ last_usage_observed_at: 123 }); | ||
| expect( | ||
| db | ||
| .prepare("SELECT COUNT(*) AS count FROM schema_migrations WHERE version = 80") | ||
| .get(), | ||
| ).toEqual({ count: 1 }); | ||
| } finally { | ||
| closeQuietly(db); | ||
| } | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2818,6 +2818,28 @@ | |
| `); | ||
| }, | ||
| }, | ||
| { | ||
| // Temporary merge-order reservation: PR #340 owns v79, so this PR must | ||
| // remain v80 even while v79 is absent from this worktree. | ||
| version: 80, | ||
| description: "persist the original observation time for tokenless usage TTL", | ||
| up(db: Database): void { | ||
| if (!tableExists(db, "session_meta")) return; | ||
| ensureColumn( | ||
| db, | ||
| "session_meta", | ||
| "last_usage_observed_at", | ||
| "INTEGER NOT NULL DEFAULT 0", | ||
| ); | ||
| db.exec(` | ||
| UPDATE session_meta | ||
| SET last_usage_observed_at = last_response_time | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When an existing session received a tokenless response after its last usage sample, this migration records that later response time as the usage-observation time. The restored sample can then pass the TTL check and drive pressure decisions beyond its real freshness window; do not mark legacy usage fresh from a response-only timestamp unless the legacy event is known to contain usage, otherwise expire the legacy sample conservatively. Prompt for AI agents |
||
| WHERE last_usage_observed_at = 0 | ||
| AND last_input_tokens > 0 | ||
| AND last_response_time > 0; | ||
| `); | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| /** | ||
|
|
@@ -2980,7 +3002,7 @@ | |
| ); | ||
| throw new Error( | ||
| `Migration v${version} failed: ${error instanceof Error ? error.message : String(error)}. Database may need manual repair.`, | ||
| ); | ||
|
Check failure on line 3005 in packages/plugin/src/features/magic-context/migrations.ts
|
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: When this PR is applied before PR #340, v80 becomes the high-water mark and the later v79 migration is skipped permanently. Land v79 first, or change migration selection to support out-of-order pending versions.
Prompt for AI agents