Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 143 additions & 1 deletion apps/desktop/src/entities/session/session-projections-store.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { act, render } from "@testing-library/react";
import { Profiler } from "react";
import { describe, expect, it } from "vitest";
import type {
AgentRuntimeEventEntry,
Expand All @@ -13,11 +14,15 @@ import {
createSessionProjection,
type SessionProjection,
} from "@/entities/session/session-projection";
import { createSessionProjectionsStore } from "@/entities/session/session-projections-store";
import {
createSessionProjectionsStore,
type SessionProjectionsStore,
} from "@/entities/session/session-projections-store";
import {
SessionProjectionsProvider,
useLiveSession,
useSessionProjections,
useViewedSession,
} from "@/entities/session/use-session-projections";

type Listener<T extends unknown[]> = (...args: T) => void;
Expand Down Expand Up @@ -553,3 +558,140 @@ describe("Session Projections store", () => {
});
});
});

function assistantMessage(id: string, piSessionId: string, seq: number): PiRuntimeEvent {
return {
id,
piSessionId,
kind: "message",
role: "assistant",
body: `Reply ${seq}`,
timestamp: `2026-09-24T10:00:0${seq}.000Z`,
};
}

describe("viewed Sessions", () => {
it("reads results as they land on a viewed Session and keeps them unread once it is released", () => {
const fake = createFakeBridge();
const store = createStore(fake);
store.insert(createSessionProjection({
id: "viewed",
projectId: "pig",
initialPrompt: "Prompt",
createdAt: "2026-09-24T09:00:00.000Z",
}));
store.apply("viewed", {
type: "runtime-bound",
stage: "starting runtime",
runtimeId: "runtime:viewed",
piSessionId: "pi:viewed",
occurredAt: "2026-09-24T09:00:01.000Z",
});

const notifies: string[] = [];
store.subscribe(() => notifies.push("notify"));
const release = store.view("viewed");

fake.emitLegacy(assistantMessage("m1", "pi:viewed", 1));

// The unread write lands inside the event's own commit, not after it.
expect(store.get("viewed")?.unreadResult).toBe(false);
expect(notifies).toHaveLength(1);

release();
fake.emitLegacy(assistantMessage("m2", "pi:viewed", 2));

expect(store.get("viewed")?.unreadResult).toBe(true);
expect(notifies).toHaveLength(2);
});

it("viewing an unread Session reads it once", () => {
const fake = createFakeBridge();
const store = createStore(fake);
store.insert({ ...boundSession("unread"), unreadResult: true });

const notifies: string[] = [];
store.subscribe(() => notifies.push("notify"));
store.view("unread");

expect(store.get("unread")?.unreadResult).toBe(false);
expect(notifies).toHaveLength(1);
});

it("reads a Session that arrives after its view started", () => {
const store = createStore();
const release = store.view("late");

// Insert bypasses apply(): the read marker has to come from commit itself.
store.insert({ ...boundSession("late"), unreadResult: true });

expect(store.get("late")?.unreadResult).toBe(false);
release();
});

it("reads a viewed Session that rehydrate brings in unread", async () => {
const fake = createFakeBridge();
const store = createStore(fake, async () => [
coldSession("hydrated", { unreadResult: true }),
]);
const release = store.view("hydrated");

await store.rehydrate();

expect(store.get("hydrated")?.unreadResult).toBe(false);
release();
});

it("commits once per assistant event while the Session is on screen", () => {
const fake = createFakeBridge();
let store!: SessionProjectionsStore;
let commits = 0;

function LiveView() {
const { projection } = useLiveSession("onscreen");
useViewedSession("onscreen");
return <span data-testid="unread">{String(projection?.unreadResult)}</span>;
}
function StoreProbe() {
store = useSessionProjections().store;
return null;
}

render(
<SessionProjectionsProvider bridge={fake.bridge}>
<StoreProbe />
<Profiler id="live" onRender={() => { commits += 1; }}>
<LiveView />
</Profiler>
</SessionProjectionsProvider>,
);

act(() => {
store.insert(createSessionProjection({
id: "onscreen",
projectId: "pig",
initialPrompt: "Prompt",
createdAt: "2026-09-24T09:00:00.000Z",
}));
store.apply("onscreen", {
type: "runtime-bound",
stage: "starting runtime",
runtimeId: "runtime:onscreen",
piSessionId: "pi:onscreen",
occurredAt: "2026-09-24T09:00:01.000Z",
});
});
commits = 0;

for (let seq = 1; seq <= 5; seq += 1) {
act(() => {
fake.emitLegacy(assistantMessage(`m${seq}`, "pi:onscreen", seq));
});
}

// One commit per event: the old pattern added a second commit per event
// from a passive effect writing "read" back after each render.
expect(commits).toBe(5);
expect(store.get("onscreen")?.unreadResult).toBe(false);
});
});
49 changes: 47 additions & 2 deletions apps/desktop/src/entities/session/session-projections-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export type SessionProjectionsStore = {
remove(sessionId: string): void;
rehydrate(): Promise<void>;
subscribe(listener: () => void): () => void;
/**
* Marks a Session as on screen until the returned release runs. Its results
* are read as they land, in the same store commit, so a view never writes
* "read" back after rendering (which cost one extra commit per event).
*/
view(sessionId: string): () => void;
};

// A projection this process has advanced past its persisted record. Replacing
Expand Down Expand Up @@ -63,16 +69,30 @@ export function createSessionProjectionsStore(options: {
let projections: SessionProjection[] = [];
const listeners = new Set<() => void>();
const subscriptions = new Map<string, { piSessionId: string; release: () => void }>();
// Sessions currently on screen, ref-counted per `view()` caller. Their
// results are already consumed by definition, so `seen` folds the read
// marker into the same projection write instead of a second apply later.
const viewed = new Map<string, number>();
const history = new Map<
string,
{ key: string; piSessionId: string; state: Exclude<SessionHistoryState, "idle"> }
>();

const seen = (projection: SessionProjection) =>
viewed.has(projection.id) && projection.unreadResult
? applySessionProjectionEvent(projection, {
type: "latest-message-rendered",
occurredAt: new Date().toISOString(),
})
: projection;

const notify = () => {
for (const listener of listeners) listener();
};
const commit = (next: SessionProjection[]) => {
projections = next;
// Every write passes here, so a viewed Session is read however its
// projection arrives (event, history read, insert, rehydrate).
projections = viewed.size > 0 ? next.map(seen) : next;
notify();
};
const get = (sessionId: string) =>
Expand Down Expand Up @@ -119,7 +139,7 @@ export function createSessionProjectionsStore(options: {

// The reducer throws on events it cannot place (an unknown queued id);
// that propagates before anything is stored.
const next = applySessionProjectionEvent(current, event);
const next = seen(applySessionProjectionEvent(current, event));
replace(next);

if (event.type === "runtime-bound") {
Expand Down Expand Up @@ -247,5 +267,30 @@ export function createSessionProjectionsStore(options: {
listeners.delete(listener);
};
},
view(sessionId) {
viewed.set(sessionId, (viewed.get(sessionId) ?? 0) + 1);
// An already-unread Session is caught up now: one notify, only on view
// start — nothing writes "read" back while events land afterwards.
const current = get(sessionId);
if (current) {
const next = seen(current);
if (next !== current) {
replace(next);
Comment on lines +270 to +278

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Late session arrival stays unread

If view() starts before an unread Session is inserted, it cannot mark that Session read. insert bypasses seen, so its unread badge remains while the Session is on screen.

Learn more

A view can start while its Session is absent from the store. view() records the id but only clears unread on the projection available at that moment. A later insert commits its projection unchanged. useViewedSession depends on the id and store, not on the projection, so no second view call clears the badge. The previous read effect depended on liveProjection?.unreadResult, so it handled a projection that appeared after the view mounted.

Example: A view starts for Session s before its projection exists. An unread projection for s is then inserted. Its unread indicator remains visible even though the Session is open.

Recommended fix: Apply seen() to projections entering through insert() while their id is viewed. Also check replacement paths such as rehydrate() for projections that can enter the store unread after a view starts.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid, thanks. Fixed in 8358f65: commit() is now the single place that applies the read marker, so insert() and rehydrate() are covered too. Added red-first tests for a Session inserted after its view started and one rehydrated unread.

}
}
let released = false;
return () => {
if (released) {
return;
}
released = true;
const count = (viewed.get(sessionId) ?? 0) - 1;
if (count <= 0) {
viewed.delete(sessionId);
} else {
viewed.set(sessionId, count);
}
};
},
};
}
15 changes: 15 additions & 0 deletions apps/desktop/src/entities/session/use-session-projections.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -304,3 +304,18 @@ export function useLiveSession(sessionId: string | null) {

return { projection, history, apply, retryHistory };
}

/** Keeps a Session read while it is on screen (SessionProjectionsStore.view). */
export function useViewedSession(sessionId: string | null) {
const context = useContext(SessionProjectionsStoreContext);

if (!context) {
throw new Error("useViewedSession requires SessionProjectionsProvider");
}

const { store } = context;
useEffect(
() => (sessionId ? store.view(sessionId) : undefined),
[sessionId, store],
);
}
54 changes: 54 additions & 0 deletions apps/desktop/src/shared/ui/astryx-text-ref.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { render } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import { Heading, Text } from "@astryxdesign/core/Text";

/**
* Guard for the @astryxdesign/core patch: Text/Heading merge their forwarded
* ref, useTruncation's callback ref, and an internal object ref into one
* callback built fresh on every render. A new callback identity makes React
* detach+reattach the ref on every commit — and useTruncation's attach calls
* setState inside the commit, which is what let nested updates accumulate
* into React #185 under dense Session events.
*/
describe("Astryx ref stability", () => {
it("Text keeps the forwarded callback ref attached across re-renders", () => {
const spy = vi.fn();
function Parent({ label }: { label: string }) {
return <Text ref={spy}>{label}</Text>;
}

const { rerender } = render(<Parent label="a" />);
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenLastCalledWith(expect.any(HTMLElement));

rerender(<Parent label="b" />);
rerender(<Parent label="c" />);
rerender(<Parent label="d" />);

// No detach (spy(null)) and no re-attach: identity must be stable.
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).not.toHaveBeenCalledWith(null);
});

it("Heading keeps the forwarded callback ref attached across re-renders", () => {
const spy = vi.fn();
function Parent({ label }: { label: string }) {
return (
<Heading level={2} ref={spy}>
{label}
</Heading>
);
}

const { rerender } = render(<Parent label="a" />);
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenLastCalledWith(expect.any(HTMLElement));

rerender(<Parent label="b" />);
rerender(<Parent label="c" />);
rerender(<Parent label="d" />);

expect(spy).toHaveBeenCalledTimes(1);
expect(spy).not.toHaveBeenCalledWith(null);
});
});
25 changes: 25 additions & 0 deletions apps/desktop/src/shared/ui/chat/chat-inline-pager.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Profiler } from "react";
import { act, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { ChatInlinePager } from "@/shared/ui/chat/chat-inline-pager";
Expand Down Expand Up @@ -104,6 +105,30 @@ describe("ChatInlinePager", () => {
expect(screen.getByText("Running bash script…")).toBeInTheDocument();
});

// A parent re-render with the same page key used to cost a second commit
// from the pager's passive effect — under dense Session events that extra
// commit per event is what let React's nested-update counter overflow (React error #185).
it("lands same-page content without an extra commit", () => {
let commits = 0;
const { rerender } = render(
<Profiler id="pager" onRender={() => { commits += 1; }}>
<ChatInlinePager pageKey="running:1">Running bash…</ChatInlinePager>
</Profiler>,
);

commits = 0;
for (const label of ["Running bash script…", "Running bash -lc…", "Running test…"]) {
rerender(
<Profiler id="pager" onRender={() => { commits += 1; }}>
<ChatInlinePager pageKey="running:1">{label}</ChatInlinePager>
</Profiler>,
);
}

expect(commits).toBe(3);
expect(screen.getByText("Running test…")).toBeInTheDocument();
});

it("swaps pages without motion when reduced motion is preferred", () => {
vi.useFakeTimers();
useReducedMotion();
Expand Down
6 changes: 3 additions & 3 deletions apps/desktop/src/shared/ui/chat/chat-inline-pager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ export function ChatInlinePager({
latestRef.current = { key: pageKey, node: children };

// Same page, new content (streaming args, a ticking duration): land it in
// place rather than turning the page on itself.
// place rather than turning the page on itself. Rendered straight from
// children below, so a parent re-render costs no extra commit.
if (pageKey === shownRef.current.key) {
shownRef.current = latestRef.current;
setShown(latestRef.current);
return;
}

Expand Down Expand Up @@ -122,7 +122,7 @@ export function ChatInlinePager({
}
}}
>
{shown.node}
{pageKey === shown.key ? children : shown.node}
</span>
</span>
</span>
Expand Down
Loading
Loading