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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ follows [Semantic Versioning](https://semver.org/).
summaries to a cheaper/faster model. Unresolvable specs fall back to the
session model with a warning. (#26)

### Fixed

- Exit summaries whose every section is "None." (trivial sessions with nothing
worth recording) are no longer persisted to the daily log, completing the
curated-write gate — previously such boilerplate entries were appended,
re-injected at every session start, and indexed by qmd. (#26)
- Exit-summary generation on `session_shutdown` is now bounded by a
self-imposed timeout (`PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS`, default 10s).
Pi core awaits shutdown handlers with no timeout, so a hanging provider
previously blocked quitting indefinitely. On expiry nothing is persisted.
(#26)

### Changed

- Reduced pull-request CI duplication and setup overhead by consolidating the
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ This ensures in-progress context survives compaction and is visible in the next
| `PI_MEMORY_SUMMARIZE_TRANSITIONS` | `1`, `true`, `yes`, `on` | unset | Also write exit summaries during lifecycle transitions (`/reload`, `/new`, `/resume`, `/fork`). By default these transitions skip summaries for speed. |
| `PI_MEMORY_EXIT_SUMMARY` | `0`, `off`, `false`, `no` to disable | unset (enabled) | Disable the exit summary on real quit (Ctrl+D, `/quit`, session end). Quitting then does no LLM call and no `qmd update`, so it is instant; explicit `memory_write` during sessions is unaffected. |
| `PI_MEMORY_EXIT_SUMMARY_MODEL` | `provider/model-id` | unset (session model) | Model used to write the exit summary, e.g. a cheaper/faster one. Unresolvable specs fall back to the session model with a warning. |
| `PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS` | positive integer (milliseconds) | `10000` | Self-imposed timeout for exit-summary generation on quit. Pi awaits shutdown handlers with no timeout, so a hanging provider would otherwise block quitting indefinitely. On expiry nothing is persisted. |

## Troubleshooting

Expand Down
46 changes: 44 additions & 2 deletions index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,35 @@ export function isExitSummaryEnabled(): boolean {
return !(value === "0" || value === "off" || value === "false" || value === "no");
}

/**
* True when a generated exit summary carries no actual content — every section
* is empty or "None.". The summary prompt instructs the model to write "None."
* under each heading when nothing is worth recording; persisting those blocks
* would pollute the daily log (re-injected every session start) and the qmd
* index with boilerplate.
*/
export function isExitSummaryEmpty(summary: string): boolean {
const contentLines = summary
.split("\n")
.map((line) => line.trim())
.filter((line) => line.length > 0 && !line.startsWith("#"));
if (contentLines.length === 0) return true;
return contentLines.every((line) => /^none\.?$/i.test(line.replace(/^[-*+]\s*/, "")));
}

const DEFAULT_EXIT_SUMMARY_TIMEOUT_MS = 10_000;

/**
* Self-imposed timeout for the exit-summary work on session_shutdown. Pi core
* awaits shutdown handlers with no timeout, and generateExitSummary() is only
* bounded by the provider's own timeout — a hanging provider would otherwise
* block quitting indefinitely. Override with PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS.
*/
export function getExitSummaryTimeoutMs(): number {
const configured = Number(process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS);
return Number.isInteger(configured) && configured > 0 ? configured : DEFAULT_EXIT_SUMMARY_TIMEOUT_MS;
}

export function shouldSkipExitSummaryForReason(reason: string | undefined): boolean {
if (!reason) return false;
if (shouldSummarizeLifecycleTransitions()) return false;
Expand Down Expand Up @@ -1458,15 +1487,26 @@ export default function (pi: ExtensionAPI) {
const reason = exitSummaryReason ?? "session-end";
exitSummaryReason = null;

let summaryTimer: ReturnType<typeof setTimeout> | undefined;
try {
if (reason) {
ensureDirs();
const result = await generateExitSummary(ctx);
// Race the summary against a self-imposed timeout: pi core awaits
// shutdown handlers with no timeout, so a hanging provider would
// otherwise block quitting indefinitely. On expiry nothing is
// persisted (the late result, if any, is simply dropped).
const summaryWork = generateExitSummary(ctx);
const expired = new Promise<null>((resolve) => {
summaryTimer = setTimeout(() => resolve(null), getExitSummaryTimeoutMs());
});
const result = await Promise.race([summaryWork, expired]);
// Only persist real summaries. The old fallback appended an
// all-"None." boilerplate block on every failed summarization
// (no API key, empty response, …), polluting the daily log —
// which is then re-injected into context every session start.
if (result.hasMessages && result.summary) {
// Successful-but-empty summaries (every section "None.") are
// filtered out for the same reason.
if (result?.hasMessages && result.summary && !isExitSummaryEmpty(result.summary)) {
const summary = result.summary;
const sid = shortSessionId(ctx.sessionManager.getSessionId());
const ts = nowTimestamp();
Expand All @@ -1480,6 +1520,7 @@ export default function (pi: ExtensionAPI) {
}
}
} finally {
if (summaryTimer) clearTimeout(summaryTimer);
if (updateTimer) {
clearTimeout(updateTimer);
updateTimer = null;
Expand Down Expand Up @@ -2371,6 +2412,7 @@ export default function (pi: ExtensionAPI) {
`- PI_MEMORY_DIR: ${process.env.PI_MEMORY_DIR ? "set" : "default"}`,
`- PI_MEMORY_EXIT_SUMMARY: ${isExitSummaryEnabled() ? "enabled" : "disabled"}`,
`- PI_MEMORY_EXIT_SUMMARY_MODEL: ${process.env.PI_MEMORY_EXIT_SUMMARY_MODEL?.trim() || "session model"}`,
`- PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS: ${getExitSummaryTimeoutMs()}`,
);

return {
Expand Down
115 changes: 115 additions & 0 deletions test/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import {
ensureDirs,
ensureQmdEmbed,
forgetBlocks,
getExitSummaryTimeoutMs,
getQmdSearchTimeoutMs,
isExitSummaryEmpty,
isExitSummaryEnabled,
nowTimestamp,
parseScratchpad,
Expand Down Expand Up @@ -1662,6 +1664,119 @@ describe("lifecycle hooks", () => {
});
});

describe("isExitSummaryEmpty", () => {
test("treats all-None summaries as empty", () => {
const summary = [
"### Decisions",
"- None.",
"### Lessons Learned",
"- None.",
"### Notes",
"- None.",
"### Follow-ups",
"- None.",
].join("\n");
expect(isExitSummaryEmpty(summary)).toBe(true);
});

test("tolerates formatting variations (bullets, case, missing period)", () => {
expect(isExitSummaryEmpty("### Decisions\nNone\n### Notes\n* none.")).toBe(true);
expect(isExitSummaryEmpty("None.")).toBe(true);
expect(isExitSummaryEmpty("### Decisions\n### Notes")).toBe(true);
});

test("keeps summaries with any real content", () => {
const summary = [
"### Decisions",
"- None.",
"### Lessons Learned",
"- None.",
"### Notes",
"- User prefers dark mode.",
"### Follow-ups",
"- None.",
].join("\n");
expect(isExitSummaryEmpty(summary)).toBe(false);
expect(isExitSummaryEmpty("### Notes\n- Discussed None. vs null semantics")).toBe(false);
});
});

describe("exit summary shutdown timeout", () => {
const fourMessageBranch = () => [
{
type: "message",
message: {
role: "user",
content: [{ type: "text", text: "Please remember we chose dark mode." }],
timestamp: Date.now(),
},
},
{
type: "message",
message: {
role: "assistant",
content: [{ type: "text", text: "Noted, using it for the storage layer." }],
timestamp: Date.now(),
},
},
{
type: "message",
message: {
role: "user",
content: [{ type: "text", text: "Also migrate the config to match." }],
timestamp: Date.now(),
},
},
{
type: "message",
message: {
role: "assistant",
content: [{ type: "text", text: "Done — config migrated and tests pass." }],
timestamp: Date.now(),
},
},
];

let savedTimeout: string | undefined;
beforeEach(() => {
savedTimeout = process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS;
});
afterEach(() => {
if (savedTimeout === undefined) delete process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS;
else process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS = savedTimeout;
});

test("getExitSummaryTimeoutMs parses env with fallback to default", () => {
delete process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS;
expect(getExitSummaryTimeoutMs()).toBe(10_000);
process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS = "250";
expect(getExitSummaryTimeoutMs()).toBe(250);
process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS = "not-a-number";
expect(getExitSummaryTimeoutMs()).toBe(10_000);
process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS = "-5";
expect(getExitSummaryTimeoutMs()).toBe(10_000);
});

test("session_shutdown stays responsive when summary generation hangs", async () => {
// Pi core awaits session_shutdown handlers with no timeout; a hanging
// provider must not block quitting forever. The API-key lookup never
// resolves here, so without a self-imposed timeout this test only fails
// via bun's per-test timeout.
process.env.PI_MEMORY_EXIT_SUMMARY_TIMEOUT_MS = "50";
const getApiKey = mock(() => new Promise<string | undefined>(() => {}));
const ctx = createShutdownCtx({
branch: fourMessageBranch(),
model: { provider: "openai", id: "gpt-4o-mini" },
modelRegistry: { getApiKey },
});

await hooks.session_shutdown({ reason: "quit" }, ctx);

expect(getApiKey).toHaveBeenCalled();
expect(fs.existsSync(dailyPath(todayStr()))).toBe(false);
});
});

// -- session_before_compact --

test("session_before_compact appends handoff when scratchpad has open items", async () => {
Expand Down
Loading