fix(sessions): never let the unassigned refill drop a resolved chip
_merge_state_row overwrote an already-represented lineage unconditionally, and recovery pass 2 (the unassigned refill) merges into the same lineage-keyed table pass 1 fills. A row already carrying a RESOLVED project_id could therefore be replaced by a bare unassigned projection — deleting the chip this PR exists to restore and swapping the row's identity from the lineage tip back to its root. Reachable wherever the SQL project_assignment filter and the Python lineage projection disagree about where a compression lineage starts. They differ today on a blank `source`: _is_continuation_session treats an empty/whitespace source as "unknown, same conversation", while the lineage CTE compares LOWER(TRIM(parent.source)) = LOWER(TRIM(child.source)) and so will not walk from an assigned tip up to a blank-source root. The mixed first pass collapses the pair into one assigned row; pass 2 then hands the root back as its own "unassigned" conversation and clobbers it. The merge is now one-way: overwrite only when the incoming row ADDS a resolved assignment, or when the row on file has none to lose. Fixing the merge rather than each possible divergence — it is the one place where "already resolved" is knowable locally, and pass 2 by construction has less assignment context than pass 1.
This commit is contained in:
1 parent
283bf6a047
commit
b90d64d9cf
2 files changed
+62
-2
No files matched your search
+15
-2
@@ -8491,6 +8491,13 @@ def _load_cli_sessions_uncached(
|
||||
carrying its resolved project assignment, and the projection loop
|
||||
must read that richer row. Pass 2 needs no budget of its own (its
|
||||
query limit is the bound) and ignores the result.
|
||||
|
||||
"Upgrade" is one-way. Overwriting a row that already carries a
|
||||
RESOLVED project_id with a projection that has none would delete the
|
||||
chip this whole function exists to restore — and swap the row's
|
||||
identity from the lineage tip back to its root. Pass 2 projects the
|
||||
same conversations as pass 1 without the assignment context, so it
|
||||
must never be able to win that race.
|
||||
"""
|
||||
lineage_ids = _lineage_ids(row)
|
||||
existing = next(
|
||||
@@ -8498,8 +8505,14 @@ def _load_cli_sessions_uncached(
|
||||
None,
|
||||
)
|
||||
if existing is not None:
|
||||
existing.clear()
|
||||
existing.update(row)
|
||||
if (
|
||||
_resolved_project_id(row.get('project_id')) is not None
|
||||
or _resolved_project_id(existing.get('project_id')) is None
|
||||
):
|
||||
# Either the incoming row adds an assignment, or the row on
|
||||
# file has none to lose: overwriting cannot drop a chip.
|
||||
existing.clear()
|
||||
existing.update(row)
|
||||
for value in lineage_ids:
|
||||
represented_rows[value] = existing
|
||||
return False
|
||||
|
||||
@@ -753,6 +753,53 @@ def test_unassigned_refill_runs_when_the_mixed_window_under_delivers(
|
||||
assert sum(1 for s in sessions if s["project_id"] == "project-live") == 3
|
||||
|
||||
|
||||
def test_unassigned_refill_cannot_clobber_a_resolved_assignment(
|
||||
fake_hermes_home, tmp_path
|
||||
):
|
||||
"""Recovery pass 2 must never un-fix this PR's own headline bug.
|
||||
|
||||
Pass 2 merges its rows into the same lineage-keyed table pass 1 fills, and
|
||||
it used to overwrite an existing entry unconditionally. When the SQL
|
||||
``project_assignment='unassigned'`` filter and the Python lineage projection
|
||||
disagree about where a compression lineage starts, pass 2 hands back the
|
||||
lineage ROOT as its own "unassigned" conversation — and the unconditional
|
||||
overwrite then replaced the assigned row with it, deleting the project chip
|
||||
and swapping the session identity from the lineage tip back to the root.
|
||||
|
||||
They disagree here on a blank ``source``: ``_is_continuation_session``
|
||||
treats an empty/whitespace source as "unknown, same conversation", while the
|
||||
SQL lineage CTE compares ``LOWER(TRIM(parent.source)) = LOWER(TRIM(...))``
|
||||
and so refuses to walk from the assigned tip up to the blank-source root.
|
||||
The merge has to be safe whether or not the two ever line up exactly.
|
||||
"""
|
||||
_register_projects(tmp_path, "project-live")
|
||||
|
||||
rows = [
|
||||
# Blank source: SQL will not join this root to its assigned tip, but the
|
||||
# Python projection collapses the pair into one row keyed on both ids.
|
||||
_session(
|
||||
"chain-root",
|
||||
BASE_TS + 500,
|
||||
source="",
|
||||
ended_at=BASE_TS + 505,
|
||||
end_reason="compression",
|
||||
),
|
||||
_session(
|
||||
"chain-tip", BASE_TS + 510, parent="chain-root", project_id="project-live"
|
||||
),
|
||||
]
|
||||
# Two spare unassigned rows so the window is short and pass 2 actually runs.
|
||||
rows.extend(_session(f"plain-{index}", BASE_TS + index) for index in range(2))
|
||||
_write_state_db(fake_hermes_home / "state.db", rows)
|
||||
|
||||
by_id = {s["session_id"]: s for s in models.get_cli_sessions()}
|
||||
|
||||
# Was: "chain-tip" gone, "chain-root" present with project_id None.
|
||||
assert "chain-tip" in by_id, "pass 2 replaced the assigned row with its root"
|
||||
assert by_id["chain-tip"]["project_id"] == "project-live"
|
||||
assert "chain-root" not in by_id, "the lineage must stay one conversation"
|
||||
|
||||
|
||||
# ── Rebase guard: upstream's kanban pass shares the system-chip helper ────────
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user