Archived
fix(ui): compact session tree indentation and allow default navigation after invalid custom input
- Only increment visual branch depth after forks so long linear session histories stay in one lane instead of scrolling off-screen; lower the max visual depth cap to match. - Reset to the no-summary default when leaving an invalid custom summary choice so Navigate is never permanently disabled by a stale invalid entry.
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@jmfederico/pi-web": patch
|
||||
---
|
||||
|
||||
Keep the session tree usable across mobile and desktop by compacting branch indentation and allowing the preselected no-summary choice to navigate immediately.
|
||||
@@ -31,6 +31,24 @@ describe("session-tree-navigator interactions", () => {
|
||||
expect(onNavigate).toHaveBeenNthCalledWith(2, "side", { mode: "none" });
|
||||
});
|
||||
|
||||
it("restores the valid no-summary default after leaving an incomplete custom choice", async () => {
|
||||
const navigator = initializedNavigator();
|
||||
const onNavigate = vi.fn<NavigateCallback>().mockResolvedValue({ cancelled: false });
|
||||
navigator.onNavigate = onNavigate;
|
||||
|
||||
clickTreeNavigate(navigator);
|
||||
callSummaryModeMethod(navigator, "custom");
|
||||
await callPromiseMethod(navigator, "submitNavigation");
|
||||
expect(onNavigate).not.toHaveBeenCalled();
|
||||
|
||||
callVoidMethod(navigator, "returnToTree");
|
||||
clickTreeNavigate(navigator);
|
||||
|
||||
expect(componentProperty(navigator, "summaryMode")).toBe("none");
|
||||
await callPromiseMethod(navigator, "submitNavigation");
|
||||
expect(onNavigate).toHaveBeenCalledWith("active", { mode: "none" });
|
||||
});
|
||||
|
||||
it("submits trimmed custom focus, exposes busy cancellation, and returns to the same node", async () => {
|
||||
const navigation = deferred<SessionTreeNavigateResult>();
|
||||
const navigator = initializedNavigator();
|
||||
@@ -137,8 +155,8 @@ describe("session-tree-navigator interactions", () => {
|
||||
expect(sessionTreeEntryReturnsToEditor("assistant")).toBe(false);
|
||||
expect(sessionTreeEntryReturnsToEditor("tool-result")).toBe(false);
|
||||
expect(sessionTreeVisualDepth(-1)).toBe(0);
|
||||
expect(sessionTreeVisualDepth(12)).toBe(12);
|
||||
expect(sessionTreeVisualDepth(20_000)).toBe(32);
|
||||
expect(sessionTreeVisualDepth(7)).toBe(7);
|
||||
expect(sessionTreeVisualDepth(20_000)).toBe(8);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ import { SESSION_TREE_CUSTOM_INSTRUCTIONS_MAX_LENGTH } from "../../../shared/api
|
||||
import { buildSessionTreeModel, initialSessionTreeSelection, toggleSessionTreeFold, transitionSessionTreeKey, validateSessionTreeSummaryChoice, visibleSessionTreeRows, type SessionTreeModel, type SessionTreeRow } from "../sessionTreeModel";
|
||||
|
||||
const EMPTY_TREE: SessionTreeSnapshot = { nodes: [], activeLeafId: null, activePathIds: [] };
|
||||
const MAX_SESSION_TREE_VISUAL_DEPTH = 32;
|
||||
const MAX_SESSION_TREE_VISUAL_DEPTH = 8;
|
||||
type NavigatorStep = "tree" | "confirm";
|
||||
type PendingFocus = "tree" | "summary" | "custom";
|
||||
|
||||
@@ -103,11 +103,11 @@ export class SessionTreeNavigator extends LitElement {
|
||||
row.activeLeaf ? "active-leaf" : "",
|
||||
isBookkeepingKind(row.node.kind) ? "bookkeeping" : "",
|
||||
].filter((value) => value !== "").join(" ");
|
||||
const visualDepth = sessionTreeVisualDepth(row.depth);
|
||||
const visualDepth = sessionTreeVisualDepth(row.branchDepth);
|
||||
return html`
|
||||
<div
|
||||
class=${classes}
|
||||
style=${`--tree-indent: ${String(visualDepth * 22)}px; --tree-indent-mobile: ${String(visualDepth * 16)}px;`}
|
||||
style=${`--tree-indent: ${String(visualDepth * 16)}px; --tree-indent-mobile: ${String(visualDepth * 12)}px;`}
|
||||
role="treeitem"
|
||||
aria-level=${String(row.depth + 1)}
|
||||
aria-selected=${selected ? "true" : "false"}
|
||||
@@ -124,16 +124,18 @@ export class SessionTreeNavigator extends LitElement {
|
||||
aria-hidden="true"
|
||||
@click=${(event: MouseEvent) => { this.toggleNode(row.node.id, event); }}
|
||||
>${row.childIds.length === 0 ? "·" : expanded ? "▾" : "▸"}</span>
|
||||
<span class="kind">${sessionTreeKindLabel(row.node.kind)}</span>
|
||||
<span class="metadata">
|
||||
<span class="kind">${sessionTreeKindLabel(row.node.kind)}</span>
|
||||
<span class="badges">
|
||||
${row.activePath && !row.activeLeaf ? html`<span class="badge path">Active path</span>` : null}
|
||||
${row.activeLeaf ? html`<span class="badge leaf">Active leaf</span>` : null}
|
||||
</span>
|
||||
</span>
|
||||
<span class="entry">
|
||||
<span class="summary" dir="auto">${row.node.summary}</span>
|
||||
${row.node.label === undefined ? null : html`<span class="label" title=${row.node.label}>${row.node.label}</span>`}
|
||||
${row.node.timestamp === undefined ? null : html`<time datetime=${row.node.timestamp}>${row.node.timestamp}</time>`}
|
||||
</span>
|
||||
<span class="badges">
|
||||
${row.activePath ? html`<span class="badge path">Active path</span>` : null}
|
||||
${row.activeLeaf ? html`<span class="badge leaf">Active leaf</span>` : null}
|
||||
</span>
|
||||
</div>
|
||||
`;
|
||||
}
|
||||
@@ -212,7 +214,6 @@ export class SessionTreeNavigator extends LitElement {
|
||||
`;
|
||||
}
|
||||
|
||||
const validation = validateSessionTreeSummaryChoice(this.summaryMode, this.customInstructions);
|
||||
const summarizing = this.summaryMode !== "none";
|
||||
return html`
|
||||
<footer>
|
||||
@@ -221,7 +222,7 @@ export class SessionTreeNavigator extends LitElement {
|
||||
${this.busy && summarizing ? html`
|
||||
<button class="danger" ?disabled=${this.aborting} @click=${() => { void this.abortNavigation(); }}>${this.aborting ? "Cancelling…" : "Cancel summarization"}</button>
|
||||
` : null}
|
||||
<button class="primary" ?disabled=${this.busy || this.selectedId === undefined || !validation.ok} @click=${() => { void this.submitNavigation(); }}>
|
||||
<button class="primary" ?disabled=${this.busy || this.selectedId === undefined} @click=${() => { void this.submitNavigation(); }}>
|
||||
${this.busy ? summarizing ? "Summarizing…" : "Navigating…" : summarizing ? "Summarize and navigate" : "Navigate"}
|
||||
</button>
|
||||
</footer>
|
||||
@@ -282,6 +283,10 @@ export class SessionTreeNavigator extends LitElement {
|
||||
|
||||
private continueToConfirmation(): void {
|
||||
if (this.selectedId === undefined || !this.model.nodesById.has(this.selectedId)) return;
|
||||
if (!validateSessionTreeSummaryChoice(this.summaryMode, this.customInstructions).ok) {
|
||||
this.summaryMode = "none";
|
||||
this.customInstructions = "";
|
||||
}
|
||||
this.step = "confirm";
|
||||
this.error = "";
|
||||
this.statusMessage = "";
|
||||
@@ -315,8 +320,10 @@ export class SessionTreeNavigator extends LitElement {
|
||||
if (this.busy || this.selectedId === undefined) return;
|
||||
const validation = validateSessionTreeSummaryChoice(this.summaryMode, this.customInstructions);
|
||||
if (!validation.ok) {
|
||||
this.error = validation.error;
|
||||
this.error = "";
|
||||
this.statusMessage = "";
|
||||
this.pendingFocus = "custom";
|
||||
this.requestUpdate();
|
||||
return;
|
||||
}
|
||||
const navigate = this.onNavigate;
|
||||
@@ -457,6 +464,11 @@ export class SessionTreeNavigator extends LitElement {
|
||||
.disclosure { width: 20px; height: 28px; display: grid; place-items: center; border-radius: 5px; color: var(--pi-muted); font-size: 15px; user-select: none; }
|
||||
.disclosure:not(.leaf):hover { color: var(--pi-text); background: var(--pi-surface-hover); }
|
||||
.disclosure.leaf { opacity: .5; }
|
||||
.metadata { display: contents; }
|
||||
.tree-row > .disclosure { grid-column: 1; grid-row: 1; }
|
||||
.tree-row > .metadata > .kind { grid-column: 2; grid-row: 1; }
|
||||
.tree-row > .entry { grid-column: 3; grid-row: 1; }
|
||||
.tree-row > .metadata > .badges { grid-column: 4; grid-row: 1; }
|
||||
.kind { display: inline-flex; align-items: center; width: fit-content; border: 1px solid var(--pi-border); border-radius: 999px; padding: 2px 7px; color: var(--pi-muted); background: var(--pi-bg); font-size: 11px; font-weight: 700; white-space: nowrap; }
|
||||
.entry { min-width: 0; display: flex; align-items: baseline; gap: 8px; }
|
||||
.summary { min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; color: var(--pi-text); }
|
||||
@@ -499,14 +511,14 @@ export class SessionTreeNavigator extends LitElement {
|
||||
|
||||
@media (max-width: 760px) {
|
||||
header { padding-top: max(12px, env(safe-area-inset-top)); }
|
||||
.tree-step { padding-inline: 8px; }
|
||||
.tree-step { padding-inline: max(8px, env(safe-area-inset-left)) max(8px, env(safe-area-inset-right)); }
|
||||
.tree-intro { padding-inline: 4px; }
|
||||
.tree-row { grid-template-columns: 20px minmax(0, 1fr) auto; padding-inline-start: calc(7px + var(--tree-indent-mobile)); }
|
||||
.tree-row .kind { grid-column: 2; }
|
||||
.tree-row .entry { grid-column: 2 / 4; display: grid; gap: 3px; }
|
||||
.tree-row { grid-template-columns: 20px minmax(0, 1fr); padding-inline-start: calc(7px + min(var(--tree-indent-mobile), 48px)); }
|
||||
.tree-row > .metadata { grid-column: 2; grid-row: 1; min-width: 0; display: flex; flex-wrap: wrap; align-items: center; gap: 5px 8px; }
|
||||
.tree-row > .metadata > .badges { margin-inline-start: auto; flex-wrap: wrap; }
|
||||
.tree-row > .entry { grid-column: 2; grid-row: 2; display: grid; gap: 3px; }
|
||||
.tree-row .summary { white-space: normal; display: -webkit-box; -webkit-box-orient: vertical; -webkit-line-clamp: 3; }
|
||||
.tree-row time { display: none; }
|
||||
.badges { grid-column: 3; grid-row: 1; flex-wrap: wrap; }
|
||||
.confirmation-step { padding: 18px 12px; }
|
||||
.custom-focus, .validation-error { margin-inline-start: 0; }
|
||||
footer { flex-wrap: wrap; }
|
||||
|
||||
@@ -48,6 +48,36 @@ describe("session tree hierarchy model", () => {
|
||||
expect(visibleSessionTreeRows(model, new Set(["root"])).map((row) => row.node.id)).toEqual(["root"]);
|
||||
});
|
||||
|
||||
it("keeps linear history in one visual lane and indents only after forks", () => {
|
||||
const model = buildSessionTreeModel({
|
||||
nodes: [
|
||||
node("root", null),
|
||||
node("before-fork", "root"),
|
||||
node("fork", "before-fork"),
|
||||
node("main", "fork"),
|
||||
node("main-next", "main"),
|
||||
node("side", "fork"),
|
||||
node("nested-fork", "side"),
|
||||
node("nested-a", "nested-fork"),
|
||||
node("nested-b", "nested-fork"),
|
||||
],
|
||||
activeLeafId: "main-next",
|
||||
activePathIds: [],
|
||||
});
|
||||
|
||||
expect(visibleSessionTreeRows(model, new Set()).map((row) => [row.node.id, row.branchDepth])).toEqual([
|
||||
["root", 0],
|
||||
["before-fork", 0],
|
||||
["fork", 0],
|
||||
["main", 1],
|
||||
["main-next", 1],
|
||||
["side", 1],
|
||||
["nested-fork", 1],
|
||||
["nested-a", 2],
|
||||
["nested-b", 2],
|
||||
]);
|
||||
});
|
||||
|
||||
it("derives one coherent active path from the normalized leaf instead of trusting malformed badges", () => {
|
||||
const model = buildSessionTreeModel({
|
||||
nodes: [node("root", null), node("active", "root"), node("unrelated", "root")],
|
||||
@@ -74,6 +104,7 @@ describe("session tree hierarchy model", () => {
|
||||
|
||||
expect(model.orderedIds).toHaveLength(count);
|
||||
expect(model.depthById.get(`node-${String(count - 1)}`)).toBe(count - 1);
|
||||
expect(model.branchDepthById.get(`node-${String(count - 1)}`)).toBe(0);
|
||||
expect(model.activePathIds.size).toBe(count);
|
||||
expect(visibleSessionTreeRows(model, new Set())).toHaveLength(count);
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ export interface SessionTreeModel {
|
||||
readonly parentById: ReadonlyMap<string, string | null>;
|
||||
readonly childrenById: ReadonlyMap<string, readonly string[]>;
|
||||
readonly depthById: ReadonlyMap<string, number>;
|
||||
readonly branchDepthById: ReadonlyMap<string, number>;
|
||||
readonly activePathIds: ReadonlySet<string>;
|
||||
readonly activeLeafId: string | null;
|
||||
}
|
||||
@@ -15,6 +16,7 @@ export interface SessionTreeModel {
|
||||
export interface SessionTreeRow {
|
||||
readonly node: SessionTreeNode;
|
||||
readonly depth: number;
|
||||
readonly branchDepth: number;
|
||||
readonly parentId: string | null;
|
||||
readonly childIds: readonly string[];
|
||||
readonly activePath: boolean;
|
||||
@@ -65,17 +67,22 @@ export function buildSessionTreeModel(snapshot: SessionTreeSnapshot): SessionTre
|
||||
}
|
||||
|
||||
const depthById = new Map<string, number>();
|
||||
const branchDepthById = new Map<string, number>();
|
||||
const visited = new Set<string>();
|
||||
const stack = [...rootIds].reverse().map((id) => ({ id, depth: 0 }));
|
||||
const stack = [...rootIds].reverse().map((id) => ({ id, depth: 0, branchDepth: 0 }));
|
||||
while (stack.length > 0) {
|
||||
const next = stack.pop();
|
||||
if (next === undefined || visited.has(next.id)) continue;
|
||||
visited.add(next.id);
|
||||
depthById.set(next.id, next.depth);
|
||||
branchDepthById.set(next.id, next.branchDepth);
|
||||
const children = mutableChildren.get(next.id) ?? [];
|
||||
// Session entries form very deep linear chains. Only forks need another
|
||||
// visual lane; indenting every parent would push ordinary history off-screen.
|
||||
const childBranchDepth = next.branchDepth + (children.length > 1 ? 1 : 0);
|
||||
for (let index = children.length - 1; index >= 0; index -= 1) {
|
||||
const childId = children[index];
|
||||
if (childId !== undefined) stack.push({ id: childId, depth: next.depth + 1 });
|
||||
if (childId !== undefined) stack.push({ id: childId, depth: next.depth + 1, branchDepth: childBranchDepth });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,6 +93,7 @@ export function buildSessionTreeModel(snapshot: SessionTreeSnapshot): SessionTre
|
||||
rootIds.push(id);
|
||||
parentById.set(id, null);
|
||||
depthById.set(id, 0);
|
||||
branchDepthById.set(id, 0);
|
||||
}
|
||||
|
||||
const childrenById = new Map<string, readonly string[]>();
|
||||
@@ -95,7 +103,7 @@ export function buildSessionTreeModel(snapshot: SessionTreeSnapshot): SessionTre
|
||||
// cannot badge an unrelated branch or keep a cycle-closing edge active.
|
||||
const activePathIds = activeLeafId === null ? new Set<string>() : sessionTreeAncestorIds(activeLeafId, parentById);
|
||||
|
||||
return { nodesById, orderedIds, rootIds, parentById, childrenById, depthById, activePathIds, activeLeafId };
|
||||
return { nodesById, orderedIds, rootIds, parentById, childrenById, depthById, branchDepthById, activePathIds, activeLeafId };
|
||||
}
|
||||
|
||||
export function visibleSessionTreeRows(model: SessionTreeModel, foldedIds: ReadonlySet<string>): SessionTreeRow[] {
|
||||
@@ -113,6 +121,7 @@ export function visibleSessionTreeRows(model: SessionTreeModel, foldedIds: Reado
|
||||
rows.push({
|
||||
node,
|
||||
depth: model.depthById.get(id) ?? 0,
|
||||
branchDepth: model.branchDepthById.get(id) ?? 0,
|
||||
parentId: model.parentById.get(id) ?? null,
|
||||
childIds,
|
||||
activePath: model.activePathIds.has(id),
|
||||
|
||||
Reference in New Issue
Block a user