fix(desktop): render /new sessions as siblings not branches (#99648)
Salvage of PR #99679 by 686f6c61, rebased onto current main. /new and idle/daily resets keep parent_session_id for durable lineage, and the sidebar's flattenSessionsWithBranches nested on that field alone — so a platform's chats collapsed into one growing nested chain of branches even though none of them were /branch forks. The backend records the distinction on disk already (model_config._reset_from vs _branched_from, gateway/session_recovery.py:433) but list payloads strip model_config before any UI can read it. - forkParentId(): nest only genuine forks — _branched_from wins, a _reset_from parent means top-level sibling, legacy/optimistic rows with only parent_session_id keep nesting. - _session_row_dict lifts _reset_from/_branched_from out of model_config so compact list rows (which strip that heavy field) carry the distinction; tui_gateway project-tree rows project the two markers too. - Optimistic desktop /branch rows stamp _branched_from so the flat render stays correct before the authoritative row arrives. Disk lineage is unchanged. Tests: a _reset_from chain renders flat, a genuine fork still nests beside a reset sharing the same parent, and list_sessions_rich promotes both markers (also under compact_rows=True). Closes #99648.
This commit is contained in:
@@ -1724,6 +1724,7 @@ function buildOptimisticSession(
|
|||||||
model: created.info?.model ?? null,
|
model: created.info?.model ?? null,
|
||||||
output_tokens: 0,
|
output_tokens: 0,
|
||||||
parent_session_id: parentSessionId,
|
parent_session_id: parentSessionId,
|
||||||
|
...(parentSessionId ? { _branched_from: parentSessionId } : {}),
|
||||||
preview,
|
preview,
|
||||||
profile: profileKey,
|
profile: profileKey,
|
||||||
source: 'tui',
|
source: 'tui',
|
||||||
|
|||||||
@@ -4,16 +4,36 @@ import type { SessionInfo } from '@/types/hermes'
|
|||||||
|
|
||||||
import { makeSessionInfo } from '../test/session-info'
|
import { makeSessionInfo } from '../test/session-info'
|
||||||
|
|
||||||
import { flattenSessionsWithBranches } from './session-branch-tree'
|
import { flattenSessionsWithBranches, forkParentId } from './session-branch-tree'
|
||||||
|
|
||||||
const session = (id: string, overrides: Partial<SessionInfo> = {}): SessionInfo =>
|
const session = (id: string, overrides: Partial<SessionInfo> = {}): SessionInfo =>
|
||||||
makeSessionInfo({ id, message_count: 1, source: 'cli', title: id, ...overrides })
|
makeSessionInfo({ id, message_count: 1, source: 'cli', title: id, ...overrides })
|
||||||
|
|
||||||
|
const fork = (id: string, parentId: string, overrides: Partial<SessionInfo> = {}): SessionInfo =>
|
||||||
|
session(id, { _branched_from: parentId, parent_session_id: parentId, ...overrides })
|
||||||
|
|
||||||
|
const reset = (id: string, parentId: string, overrides: Partial<SessionInfo> = {}): SessionInfo =>
|
||||||
|
session(id, { _reset_from: parentId, parent_session_id: parentId, ...overrides })
|
||||||
|
|
||||||
|
describe('forkParentId', () => {
|
||||||
|
it('returns undefined for /new and idle/daily reset lineage', () => {
|
||||||
|
expect(forkParentId(reset('next', 'prev'))).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns the branch parent for a genuine /branch fork', () => {
|
||||||
|
expect(forkParentId(fork('branch', 'parent'))).toBe('parent')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('falls back to parent_session_id for legacy/optimistic forks without the marker', () => {
|
||||||
|
expect(forkParentId(session('branch', { parent_session_id: 'parent' }))).toBe('parent')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe('flattenSessionsWithBranches', () => {
|
describe('flattenSessionsWithBranches', () => {
|
||||||
it('nests branch rows under their parent with tree stems', () => {
|
it('nests branch rows under their parent with tree stems', () => {
|
||||||
const parent = session('parent', { last_active: 20 })
|
const parent = session('parent', { last_active: 20 })
|
||||||
const branchA = session('branch-a', { last_active: 15, parent_session_id: 'parent' })
|
const branchA = fork('branch-a', 'parent', { last_active: 15 })
|
||||||
const branchB = session('branch-b', { last_active: 10, parent_session_id: 'parent' })
|
const branchB = fork('branch-b', 'parent', { last_active: 10 })
|
||||||
|
|
||||||
expect(flattenSessionsWithBranches([parent, branchA, branchB])).toEqual([
|
expect(flattenSessionsWithBranches([parent, branchA, branchB])).toEqual([
|
||||||
{ session: parent },
|
{ session: parent },
|
||||||
@@ -24,7 +44,7 @@ describe('flattenSessionsWithBranches', () => {
|
|||||||
|
|
||||||
it('follows a compressed parent via lineage root id', () => {
|
it('follows a compressed parent via lineage root id', () => {
|
||||||
const tip = session('tip', { _lineage_root_id: 'root', last_active: 30 })
|
const tip = session('tip', { _lineage_root_id: 'root', last_active: 30 })
|
||||||
const branch = session('branch', { parent_session_id: 'root', last_active: 10 })
|
const branch = fork('branch', 'root', { last_active: 10 })
|
||||||
|
|
||||||
expect(flattenSessionsWithBranches([tip, branch])).toEqual([
|
expect(flattenSessionsWithBranches([tip, branch])).toEqual([
|
||||||
{ session: tip },
|
{ session: tip },
|
||||||
@@ -98,7 +118,7 @@ describe('flattenSessionsWithBranches', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
it('keeps orphan branches at the top level when the parent is missing', () => {
|
it('keeps orphan branches at the top level when the parent is missing', () => {
|
||||||
const branch = session('branch', { parent_session_id: 'missing' })
|
const branch = fork('branch', 'missing')
|
||||||
|
|
||||||
expect(flattenSessionsWithBranches([branch])).toEqual([{ session: branch }])
|
expect(flattenSessionsWithBranches([branch])).toEqual([{ session: branch }])
|
||||||
})
|
})
|
||||||
@@ -119,7 +139,7 @@ describe('flattenSessionsWithBranches', () => {
|
|||||||
it("preserveOrder keeps the caller's root order even when activity is newer lower down", () => {
|
it("preserveOrder keeps the caller's root order even when activity is newer lower down", () => {
|
||||||
const important = session('important', { last_active: 10 })
|
const important = session('important', { last_active: 10 })
|
||||||
const background = session('background', { last_active: 99 })
|
const background = session('background', { last_active: 99 })
|
||||||
const branch = session('branch', { last_active: 50, parent_session_id: 'important' })
|
const branch = fork('branch', 'important', { last_active: 50 })
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
flattenSessionsWithBranches([important, background, branch], { preserveOrder: true }).map(e => ({
|
flattenSessionsWithBranches([important, background, branch], { preserveOrder: true }).map(e => ({
|
||||||
@@ -132,4 +152,38 @@ describe('flattenSessionsWithBranches', () => {
|
|||||||
{ id: 'background', stem: undefined }
|
{ id: 'background', stem: undefined }
|
||||||
])
|
])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('renders /new and idle/daily resets as siblings, not nested branches', () => {
|
||||||
|
const first = session('first', { last_active: 10 })
|
||||||
|
const second = reset('second', 'first', { last_active: 20 })
|
||||||
|
const third = reset('third', 'second', { last_active: 30 })
|
||||||
|
|
||||||
|
expect(flattenSessionsWithBranches([first, second, third]).map(e => ({ id: e.session.id, stem: e.branchStem }))).toEqual([
|
||||||
|
{ id: 'third', stem: undefined },
|
||||||
|
{ id: 'second', stem: undefined },
|
||||||
|
{ id: 'first', stem: undefined }
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('still nests a genuine fork when a reset sibling shares the same parent_session_id', () => {
|
||||||
|
const parent = session('parent', { last_active: 10 })
|
||||||
|
const branch = fork('branch', 'parent', { last_active: 15 })
|
||||||
|
const nextTopic = reset('next', 'parent', { last_active: 20 })
|
||||||
|
|
||||||
|
expect(flattenSessionsWithBranches([parent, branch, nextTopic]).map(e => ({ id: e.session.id, stem: e.branchStem }))).toEqual([
|
||||||
|
{ id: 'next', stem: undefined },
|
||||||
|
{ id: 'parent', stem: undefined },
|
||||||
|
{ id: 'branch', stem: '└─ ' }
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('nests a parent_session_id-only row (optimistic / legacy fork) when _reset_from is absent', () => {
|
||||||
|
const parent = session('parent', { last_active: 20 })
|
||||||
|
const branch = session('branch', { last_active: 10, parent_session_id: 'parent' })
|
||||||
|
|
||||||
|
expect(flattenSessionsWithBranches([parent, branch])).toEqual([
|
||||||
|
{ session: parent },
|
||||||
|
{ branchStem: '└─ ', session: branch }
|
||||||
|
])
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -17,6 +17,28 @@ export interface FlattenSessionsOptions {
|
|||||||
|
|
||||||
const recency = (session: SessionInfo): number => session.last_active || session.started_at || 0
|
const recency = (session: SessionInfo): number => session.last_active || session.started_at || 0
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Parent id to nest under, or undefined for a top-level sibling.
|
||||||
|
*
|
||||||
|
* `/new` and idle/daily rotation keep `parent_session_id` for durable lineage
|
||||||
|
* but stamp `_reset_from`. Those are new conversations, not nested forks.
|
||||||
|
* Genuine `/branch` writes `_branched_from`. Optimistic desktop branch rows
|
||||||
|
* (and legacy forks minted before the marker) only have `parent_session_id`.
|
||||||
|
*/
|
||||||
|
export function forkParentId(session: SessionInfo): string | undefined {
|
||||||
|
if (session._reset_from?.trim()) {
|
||||||
|
return undefined
|
||||||
|
}
|
||||||
|
|
||||||
|
const branchedFrom = session._branched_from?.trim()
|
||||||
|
|
||||||
|
if (branchedFrom) {
|
||||||
|
return branchedFrom
|
||||||
|
}
|
||||||
|
|
||||||
|
return session.parent_session_id?.trim() || undefined
|
||||||
|
}
|
||||||
|
|
||||||
// Profile-qualified compression lineage, same key as mergeSessionPage (store/session.ts). The
|
// Profile-qualified compression lineage, same key as mergeSessionPage (store/session.ts). The
|
||||||
// backend's `_lineage_root_id` follows compression edges only; /branch and /new children get
|
// backend's `_lineage_root_id` follows compression edges only; /branch and /new children get
|
||||||
// their own root, so rows sharing a key are one conversation, never a fork.
|
// their own root, so rows sharing a key are one conversation, never a fork.
|
||||||
@@ -97,7 +119,7 @@ export function flattenSessionsWithBranches(
|
|||||||
const nestedIds = new Set<string>()
|
const nestedIds = new Set<string>()
|
||||||
|
|
||||||
for (const session of sessions) {
|
for (const session of sessions) {
|
||||||
const parentId = session.parent_session_id?.trim()
|
const parentId = forkParentId(session)
|
||||||
|
|
||||||
if (!parentId) {
|
if (!parentId) {
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -562,8 +562,15 @@ export interface SessionInfo {
|
|||||||
message_count: number
|
message_count: number
|
||||||
model: null | string
|
model: null | string
|
||||||
output_tokens: number
|
output_tokens: number
|
||||||
/** Parent conversation when this row is a /branch fork. */
|
/** Parent conversation id. Written for genuine /branch forks *and* for
|
||||||
|
* /new / idle / daily resets (durable lineage). Nesting uses
|
||||||
|
* {@link _branched_from} vs {@link _reset_from}, not this field alone. */
|
||||||
parent_session_id?: null | string
|
parent_session_id?: null | string
|
||||||
|
/** Predecessor of a /new or idle/daily reset. Not a fork — the sidebar
|
||||||
|
* renders these as siblings of the previous topic. */
|
||||||
|
_reset_from?: null | string
|
||||||
|
/** Parent of a genuine /branch fork. The sidebar nests only these. */
|
||||||
|
_branched_from?: null | string
|
||||||
/** Durable server-side pin flag (`sessions.pinned`). The list endpoints
|
/** Durable server-side pin flag (`sessions.pinned`). The list endpoints
|
||||||
* back-fill pinned conversations past their LIMIT, so a pinned row is
|
* back-fill pinned conversations past their LIMIT, so a pinned row is
|
||||||
* always present in a page — which makes this authoritative for the
|
* always present in a page — which makes this authoritative for the
|
||||||
|
|||||||
@@ -525,6 +525,29 @@ class SessionDB(
|
|||||||
resolved = data.pop(f"_{column}_resolved")
|
resolved = data.pop(f"_{column}_resolved")
|
||||||
if column in data:
|
if column in data:
|
||||||
data[column] = resolved
|
data[column] = resolved
|
||||||
|
# Lift /new vs /branch markers out of model_config so list payloads
|
||||||
|
# (which strip that heavy field) can still tell a reset sibling from
|
||||||
|
# a genuine fork. Keep this a local helper: tests sometimes replace
|
||||||
|
# the SessionDB name with a factory lambda.
|
||||||
|
if not (data.get("_reset_from") and data.get("_branched_from")):
|
||||||
|
raw = data.get("model_config")
|
||||||
|
cfg = None
|
||||||
|
if isinstance(raw, str) and raw:
|
||||||
|
try:
|
||||||
|
cfg = json.loads(raw)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
cfg = None
|
||||||
|
elif isinstance(raw, dict):
|
||||||
|
cfg = raw
|
||||||
|
if isinstance(cfg, dict):
|
||||||
|
if not data.get("_reset_from"):
|
||||||
|
value = cfg.get("_reset_from")
|
||||||
|
if isinstance(value, str) and value.strip():
|
||||||
|
data["_reset_from"] = value.strip()
|
||||||
|
if not data.get("_branched_from"):
|
||||||
|
value = cfg.get("_branched_from")
|
||||||
|
if isinstance(value, str) and value.strip():
|
||||||
|
data["_branched_from"] = value.strip()
|
||||||
return data
|
return data
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
|
|||||||
@@ -3057,6 +3057,32 @@ class TestListSessionsRich:
|
|||||||
assert "delegate" not in ids, "Delegate sub-agent should not appear in default list"
|
assert "delegate" not in ids, "Delegate sub-agent should not appear in default list"
|
||||||
assert "root" in ids
|
assert "root" in ids
|
||||||
|
|
||||||
|
def test_rich_list_promotes_reset_and_branch_markers(self, db):
|
||||||
|
"""List rows expose _reset_from / _branched_from so UIs can tell a
|
||||||
|
/new reset from a genuine /branch without reading model_config."""
|
||||||
|
db.create_session("parent", "cli")
|
||||||
|
db.create_session(
|
||||||
|
"reset_child",
|
||||||
|
"cli",
|
||||||
|
parent_session_id="parent",
|
||||||
|
model_config={"_reset_from": "parent"},
|
||||||
|
)
|
||||||
|
db.create_session(
|
||||||
|
"branch_child",
|
||||||
|
"cli",
|
||||||
|
parent_session_id="parent",
|
||||||
|
model_config={"_branched_from": "parent"},
|
||||||
|
)
|
||||||
|
|
||||||
|
by_id = {row["id"]: row for row in db.list_sessions_rich()}
|
||||||
|
assert by_id["reset_child"]["_reset_from"] == "parent"
|
||||||
|
assert not by_id["reset_child"].get("_branched_from")
|
||||||
|
assert by_id["branch_child"]["_branched_from"] == "parent"
|
||||||
|
assert not by_id["branch_child"].get("_reset_from")
|
||||||
|
compact = {row["id"]: row for row in db.list_sessions_rich(compact_rows=True)}
|
||||||
|
assert compact["reset_child"]["_reset_from"] == "parent"
|
||||||
|
assert compact["branch_child"]["_branched_from"] == "parent"
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
class TestCompressionChainProjection:
|
class TestCompressionChainProjection:
|
||||||
|
|||||||
@@ -321,7 +321,8 @@ def _project_tree_row(r: dict) -> dict:
|
|||||||
"""Project a SessionDB row to the minimal shape the sidebar renders (grouping fields +
|
"""Project a SessionDB row to the minimal shape the sidebar renders (grouping fields +
|
||||||
what ``SidebarSessionRow`` reads), minus the heavy columns."""
|
what ``SidebarSessionRow`` reads), minus the heavy columns."""
|
||||||
row = {k: r.get(k) for k in (
|
row = {k: r.get(k) for k in (
|
||||||
"id", "_lineage_root_id", "_lineage_ids", "parent_session_id", "title", "preview")}
|
"id", "_lineage_root_id", "_lineage_ids", "parent_session_id",
|
||||||
|
"_reset_from", "_branched_from", "title", "preview")}
|
||||||
row.update(
|
row.update(
|
||||||
started_at=r.get("started_at") or 0, ended_at=r.get("ended_at"),
|
started_at=r.get("started_at") or 0, ended_at=r.get("ended_at"),
|
||||||
last_active=r.get("last_active") or r.get("started_at") or 0,
|
last_active=r.get("last_active") or r.get("started_at") or 0,
|
||||||
|
|||||||
Reference in New Issue
Block a user