release: apply Opus SHOULD-FIX 1+2 + add #1372 manual-cron persistence
Opus pre-release findings on #1370 applied: SHOULD-FIX 1: Tightened parent_session_id exposure to only emit when the parent's end_reason is in {compression, cli_close}. Without this, two distinct WebUI sessions sharing a non-continuation parent (e.g. 'user_stop') would get clustered by frontend's _sessionLineageKey (which falls through to parent_session_id when _lineage_root_id is missing) and incorrectly collapsed into a single sidebar row. Updated assertions in: - tests/test_session_lineage_metadata_api.py:: test_non_compression_state_db_parent_does_not_create_sidebar_lineage - tests/test_pr1370_lineage_metadata_perf_and_orphan.py:: test_non_compression_parent_does_not_extend_lineage SHOULD-FIX 2: Chunked the IN-clause to 500 vars to stay under SQLITE_MAX_VARIABLE_NUMBER. Python 3.9 ships sqlite 3.31 with the default limit of 999. A power user with 2000+ sessions in the sidebar would hit OperationalError, the silent except-wrapper would swallow it, and lineage collapse would never work. Added test_in_clause_chunked_for_large_session_set with SQL interception to lock the invariant in source. PR addition (per user directive — Opus + my review, no second independent review round needed for combined batch): #1372 from @NocGeek — fix: persist manual cron run results. Self-contained 89 LOC fix split out from the held #1352. Mirrors the scheduled-cron path (cron/scheduler.py:1334-1364) exactly: saves output, marks job complete, treats empty response as soft failure with matching error string. 2 behavioral tests using sys.modules monkeypatch to mock cron.scheduler.run_job. CI not yet attached because branch is brand-new; ran the new tests + adjacent suites locally — all pass. Final test count: 3471 passing, 0 failed. Also adds 2 more regression tests for the perf-fix invariants: - test_in_clause_chunked_for_large_session_set - test_two_children_sharing_non_continuation_parent_not_collapsed
This commit is contained in:
nesquena-hermes
parent
89dcab8327
commit
63251ad206
4 changed files with 136 additions and 22 deletions
|
|
@ -5,7 +5,9 @@
|
|||
## [v0.50.251] — 2026-04-30
|
||||
|
||||
### Fixed
|
||||
- **Sidebar lineage collapse now works for WebUI JSON sessions, not just imported gateway rows** — PR #1358 (v0.50.249) added the client-side lineage-collapse helper but `/api/sessions` only included `_lineage_root_id` for gateway-imported rows. WebUI JSON sessions (the common case) had no grouping key, so cross-surface continuation chains (CLI-close → WebUI continuation, or compression chains within WebUI) still rendered as separate sidebar rows. Now `/api/sessions` reads `parent_session_id` and `end_reason` from `state.db.sessions` for every WebUI session id in the sidebar payload, walks the parent chain when `end_reason in {'compression', 'cli_close'}`, and exposes `_lineage_root_id` + `_compression_segment_count`. Cycle-detected via a `seen` set; depth-bounded to 20 hops to cap pathological data. **Pre-release fix:** swapped the original full-table-scan (`SELECT id, parent_session_id, end_reason FROM sessions`) for a parameterized `WHERE id IN (...)` query that hits PRIMARY KEY + `idx_sessions_parent` — ~50× faster at 1000 rows, scales linearly. **Pre-release fix:** suppressed orphan `parent_session_id` references (parent row pruned/deleted) so the frontend's lineage helper doesn't cluster orphans into never-collapsing single-row groups. (`api/agent_sessions.py`, `api/models.py`, `tests/test_session_lineage_metadata_api.py`, `tests/test_pr1370_lineage_metadata_perf_and_orphan.py`, `tests/test_gateway_sync.py`) @dso2ng — PR #1370
|
||||
- **Sidebar lineage collapse now works for WebUI JSON sessions, not just imported gateway rows** — PR #1358 (v0.50.249) added the client-side lineage-collapse helper but `/api/sessions` only included `_lineage_root_id` for gateway-imported rows. WebUI JSON sessions (the common case) had no grouping key, so cross-surface continuation chains (CLI-close → WebUI continuation, or compression chains within WebUI) still rendered as separate sidebar rows. Now `/api/sessions` reads `parent_session_id` and `end_reason` from `state.db.sessions` for every WebUI session id in the sidebar payload, walks the parent chain when `end_reason in {'compression', 'cli_close'}`, and exposes `_lineage_root_id` + `_compression_segment_count`. Cycle-detected via a `seen` set; depth-bounded to 20 hops to cap pathological data. **Pre-release fix:** swapped the original full-table-scan for a parameterized `WHERE id IN (...)` query that hits PRIMARY KEY + `idx_sessions_parent` — ~50× faster at 1000 rows, scales linearly. **Pre-release fix:** chunked IN clause to 500 vars to stay under SQLITE_MAX_VARIABLE_NUMBER on older sqlite (Python 3.9 ships sqlite 3.31 with default limit 999) — without this a power user with 2000+ sessions in the sidebar would hit `OperationalError: too many SQL variables`, the silent except-wrapper would swallow it, and lineage collapse would never work for them. **Pre-release fix:** tightened `parent_session_id` exposure — only emitted when the parent's `end_reason` is `compression` or `cli_close` (not for `user_stop`/etc), since the frontend's `_sessionLineageKey` falls through to `parent_session_id` and would incorrectly collapse two children of a non-continuation parent into a single row. (`api/agent_sessions.py`, `api/models.py`, `tests/test_session_lineage_metadata_api.py`, `tests/test_pr1370_lineage_metadata_perf_and_orphan.py`, `tests/test_gateway_sync.py`) @dso2ng — PR #1370
|
||||
- **Manual cron runs persist output and metadata like scheduled runs** — manual WebUI cron runs called `cron.scheduler.run_job(job)` and then only cleared the in-memory running flag. The job's output was dropped (never written via `save_job_output`) and `last_run_at` / `last_status` were never updated. Now the manual-run wrapper (`_run_cron_tracked`) matches the scheduled-cron path at `cron/scheduler.py:1334-1364` exactly: saves output, marks the job complete, treats empty `final_response` as a soft failure (with the same error string), and records failures via `mark_job_run(False, str(e))`. (`api/routes.py`, `tests/test_cron_manual_run_persistence.py`) @NocGeek — PR #1372 (split out from the held #1352 per pre-release feedback)
|
||||
|
||||
|
||||
## [v0.50.250] — 2026-04-30
|
||||
|
||||
|
|
|
|||
|
|
@ -291,6 +291,15 @@ def read_session_lineage_metadata(db_path: Path, session_ids: list[str] | set[st
|
|||
# Fetch the wanted ids first, then chase parent_session_id chains
|
||||
# in batches until no new ids appear. Each batch hits PRIMARY KEY
|
||||
# so it's effectively O(N) lookups.
|
||||
#
|
||||
# IN-clause is chunked to 500 to stay under SQLITE_MAX_VARIABLE_NUMBER
|
||||
# on older sqlite (Python 3.9 ships sqlite 3.31 which defaults to 999;
|
||||
# newer Python ships sqlite 3.32+ at 32766). On a power user with
|
||||
# 2000+ sessions in the sidebar, an unchunked first hop would raise
|
||||
# `OperationalError: too many SQL variables`, get swallowed by the
|
||||
# except below, and silently disable lineage collapse forever.
|
||||
# (Opus pre-release review of v0.50.251, SHOULD-FIX 2.)
|
||||
IN_CHUNK = 500
|
||||
rows: dict[str, dict] = {}
|
||||
to_fetch = set(wanted)
|
||||
# Cap walk depth to bound worst-case query count. Real lineage
|
||||
|
|
@ -299,15 +308,17 @@ def read_session_lineage_metadata(db_path: Path, session_ids: list[str] | set[st
|
|||
for _hop in range(20):
|
||||
if not to_fetch:
|
||||
break
|
||||
placeholders = ','.join('?' * len(to_fetch))
|
||||
fetch_list = list(to_fetch)
|
||||
to_fetch = set()
|
||||
cur.execute(
|
||||
f"SELECT id, parent_session_id, end_reason FROM sessions WHERE id IN ({placeholders})",
|
||||
fetch_list,
|
||||
)
|
||||
for row in cur.fetchall():
|
||||
rows[row['id']] = dict(row)
|
||||
for i in range(0, len(fetch_list), IN_CHUNK):
|
||||
chunk = fetch_list[i:i + IN_CHUNK]
|
||||
placeholders = ','.join('?' * len(chunk))
|
||||
cur.execute(
|
||||
f"SELECT id, parent_session_id, end_reason FROM sessions WHERE id IN ({placeholders})",
|
||||
chunk,
|
||||
)
|
||||
for row in cur.fetchall():
|
||||
rows[row['id']] = dict(row)
|
||||
# Queue up parents we haven't fetched yet.
|
||||
for sid in fetch_list:
|
||||
parent_id = rows.get(sid, {}).get('parent_session_id')
|
||||
|
|
@ -323,13 +334,19 @@ def read_session_lineage_metadata(db_path: Path, session_ids: list[str] | set[st
|
|||
continue
|
||||
|
||||
parent_id = row.get('parent_session_id')
|
||||
# Only expose parent_session_id when the parent actually exists in
|
||||
# state.db. Orphan references (parent row was pruned/deleted) used to
|
||||
# leak through and the frontend would treat them as a sidebar
|
||||
# grouping key (#1358's _sessionLineageKey falls through to
|
||||
# parent_session_id when _lineage_root_id is missing). Caught during
|
||||
# pre-release review of v0.50.251.
|
||||
if parent_id and parent_id in rows:
|
||||
# Only expose parent_session_id when:
|
||||
# 1) the parent actually exists in state.db (orphan refs would
|
||||
# otherwise leak through and the frontend would treat them as
|
||||
# sidebar grouping keys via #1358's _sessionLineageKey
|
||||
# fall-through)
|
||||
# 2) the parent's end_reason is one of {compression, cli_close} —
|
||||
# i.e. only TRUE continuations. Without this, two distinct
|
||||
# WebUI sessions sharing a `user_stop` parent would get
|
||||
# collapsed into a single sidebar row by #1358's helper
|
||||
# (it groups by parent_session_id as the third-fallback key).
|
||||
# (Opus pre-release review of v0.50.251, SHOULD-FIX 1.)
|
||||
parent_row = rows.get(parent_id) if parent_id else None
|
||||
if parent_row and parent_row.get('end_reason') in {'compression', 'cli_close'}:
|
||||
metadata.setdefault(sid, {})['parent_session_id'] = parent_id
|
||||
|
||||
root_id = sid
|
||||
|
|
|
|||
|
|
@ -180,10 +180,97 @@ def test_chain_walk_returns_correct_root_for_real_compression_chain(tmp_path):
|
|||
assert result["tip"]["parent_session_id"] == "seg3"
|
||||
|
||||
|
||||
def test_in_clause_chunked_for_large_session_set(tmp_path, monkeypatch):
|
||||
"""The first hop must chunk the IN clause into batches of <= 500.
|
||||
Without chunking, a power user with 2000+ sessions in the sidebar would
|
||||
trigger SQLITE_MAX_VARIABLE_NUMBER on Python 3.9's sqlite 3.31 (default
|
||||
limit 999), the OperationalError gets swallowed by the except wrapper,
|
||||
and lineage collapse silently disables forever for that user.
|
||||
(Opus pre-release review of v0.50.251, SHOULD-FIX 2.)
|
||||
"""
|
||||
from api import agent_sessions
|
||||
|
||||
db = tmp_path / "state.db"
|
||||
conn = _make_db(db)
|
||||
# 1500 unrelated rows
|
||||
for i in range(1500):
|
||||
_insert(conn, f"session_{i:04d}")
|
||||
conn.close()
|
||||
|
||||
# Track each query's IN clause variable count
|
||||
in_counts = []
|
||||
real_connect = sqlite3.connect
|
||||
|
||||
class _TrackingConn:
|
||||
def __init__(self, *args, **kw):
|
||||
self._real = real_connect(*args, **kw)
|
||||
def cursor(self):
|
||||
return _TrackingCursor(self._real.cursor())
|
||||
def __enter__(self): return self
|
||||
def __exit__(self, *a): return self._real.__exit__(*a)
|
||||
@property
|
||||
def row_factory(self): return self._real.row_factory
|
||||
@row_factory.setter
|
||||
def row_factory(self, v): self._real.row_factory = v
|
||||
|
||||
class _TrackingCursor:
|
||||
def __init__(self, real): self._real = real
|
||||
def execute(self, sql, *args):
|
||||
if "IN (" in sql:
|
||||
in_counts.append(sql.count("?"))
|
||||
return self._real.execute(sql, *args)
|
||||
def fetchall(self): return self._real.fetchall()
|
||||
def fetchone(self): return self._real.fetchone()
|
||||
|
||||
monkeypatch.setattr(sqlite3, "connect", _TrackingConn)
|
||||
|
||||
# Request 1500 ids (the full set we inserted)
|
||||
wanted = [f"session_{i:04d}" for i in range(1500)]
|
||||
agent_sessions.read_session_lineage_metadata(db, wanted)
|
||||
|
||||
assert in_counts, "Expected at least one IN-clause query"
|
||||
over_limit = [n for n in in_counts if n > 999]
|
||||
assert not over_limit, (
|
||||
f"IN clause must be chunked to <= 999 vars to stay under SQLite default "
|
||||
f"limit (chunked to 500 in the implementation). Found queries with "
|
||||
f"{over_limit} variables — would raise OperationalError on older sqlite."
|
||||
)
|
||||
|
||||
|
||||
def test_two_children_sharing_non_continuation_parent_not_collapsed(tmp_path):
|
||||
"""The contract Opus flagged: when two distinct WebUI sessions share the
|
||||
same parent and that parent's end_reason is NOT compression/cli_close
|
||||
(e.g. 'user_stop'), neither child should expose parent_session_id.
|
||||
The frontend's #1358 lineage helper would otherwise group them under
|
||||
the parent's id key and incorrectly collapse the rows.
|
||||
"""
|
||||
from api.agent_sessions import read_session_lineage_metadata
|
||||
|
||||
db = tmp_path / "state.db"
|
||||
conn = _make_db(db)
|
||||
_insert(conn, "shared_parent", end_reason="user_stop")
|
||||
_insert(conn, "child_a", parent="shared_parent", end_reason=None)
|
||||
_insert(conn, "child_b", parent="shared_parent", end_reason=None)
|
||||
conn.close()
|
||||
|
||||
result = read_session_lineage_metadata(db, ["child_a", "child_b"])
|
||||
# Neither child should expose parent_session_id (would let frontend
|
||||
# group them under the same key and collapse one)
|
||||
for sid in ["child_a", "child_b"]:
|
||||
entry = result.get(sid, {})
|
||||
assert "parent_session_id" not in entry, (
|
||||
f"{sid} leaked parent_session_id despite parent being non-continuation"
|
||||
)
|
||||
|
||||
|
||||
def test_non_compression_parent_does_not_extend_lineage(tmp_path):
|
||||
"""If parent's end_reason is something OTHER than 'compression' or
|
||||
'cli_close' (e.g. 'user_stop', 'session_reset', 'cron_complete'),
|
||||
the chain stops at that boundary."""
|
||||
the chain stops at that boundary AND parent_session_id is not exposed
|
||||
(tightened in v0.50.251 per Opus SHOULD-FIX 1 — exposing the parent
|
||||
ref would let the frontend group two children sharing the same
|
||||
non-continuation parent into a single sidebar row).
|
||||
"""
|
||||
from api.agent_sessions import read_session_lineage_metadata
|
||||
|
||||
db = tmp_path / "state.db"
|
||||
|
|
@ -193,8 +280,11 @@ def test_non_compression_parent_does_not_extend_lineage(tmp_path):
|
|||
conn.close()
|
||||
|
||||
result = read_session_lineage_metadata(db, ["child"])
|
||||
# parent_session_id should still be exposed (parent exists)
|
||||
assert result["child"]["parent_session_id"] == "parent"
|
||||
# But _lineage_root_id should NOT be set — chain doesn't span the boundary
|
||||
assert "_lineage_root_id" not in result["child"]
|
||||
assert "_compression_segment_count" not in result["child"]
|
||||
# parent_session_id must NOT be exposed (parent's end_reason is non-continuation)
|
||||
assert "child" not in result or "parent_session_id" not in result["child"], (
|
||||
"parent_session_id leaked through despite parent's end_reason being "
|
||||
"'user_stop' — would cause incorrect sidebar collapse."
|
||||
)
|
||||
# _lineage_root_id should NOT be set — chain doesn't span the boundary
|
||||
assert "child" not in result or "_lineage_root_id" not in result["child"]
|
||||
assert "child" not in result or "_compression_segment_count" not in result["child"]
|
||||
|
|
|
|||
|
|
@ -121,7 +121,12 @@ def test_non_compression_state_db_parent_does_not_create_sidebar_lineage(_isolat
|
|||
|
||||
rows = {row["session_id"]: row for row in all_sessions()}
|
||||
|
||||
assert rows["lineage_api_plain_child"].get("parent_session_id") == "lineage_api_plain_parent"
|
||||
# parent_session_id must NOT be exposed when the parent's end_reason
|
||||
# is not a continuation (compression / cli_close). The frontend's
|
||||
# _sessionLineageKey would otherwise group two children sharing a
|
||||
# `user_stop` parent under the same key — incorrect collapse.
|
||||
# (Tightened in v0.50.251 per Opus SHOULD-FIX 1.)
|
||||
assert "parent_session_id" not in rows["lineage_api_plain_child"]
|
||||
assert "_lineage_root_id" not in rows["lineage_api_plain_child"]
|
||||
finally:
|
||||
conn.close()
|
||||
|
|
|
|||
Loading…
Reference in a new issue