Skip to content
Open
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
66 changes: 66 additions & 0 deletions packages/pi/src/effort-history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,69 @@ export function collectPiEffortHistory(

return transitions
}

type SessionEntryWithParent = SessionEntryLike & {
id: string
parentId?: string | null
firstKeptEntryId?: unknown
}

type SessionManagerLeaf = {
getLeafId?: () => string | null
[key: string]: unknown
}

export function resolveSessionLeafId(
sessionManager: SessionManagerLeaf,
): string | null | undefined {
return sessionManager.getLeafId?.()
}

/**
* Compaction-aware active entry list, mirroring the host SDK's
* buildContextEntries: latest compaction plus kept entries replace the
* summarized prefix. Implemented locally because some Pi-compatible hosts
* (oh-my-pi) lack the SDK method while sharing the entry model.
*
* `undefined` means the leaf is unknown and selects the latest entry; `null`
* means an explicit no-leaf state and returns an empty list; a string selects
* that leaf. Callers must preserve the distinction and not coalesce an absent
* leaf method into `null`.
*/
export function buildContextEntries(
entries: readonly SessionEntryWithParent[],
leafId?: string | null,
): SessionEntryWithParent[] {
const byId = new Map(entries.map((entry) => [entry.id, entry]))
let leaf: SessionEntryWithParent | undefined
if (leafId === null) return []
if (leafId) leaf = byId.get(leafId)
leaf ??= entries.at(-1)
Comment on lines +127 to +128

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When leafId is a stale or unknown string, buildContextEntries silently falls back to the latest entry and can build effort history for the wrong branch. Distinguish undefined from a string and return an empty context when the string is not present.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pi/src/effort-history.ts, line 127:

<comment>When `leafId` is a stale or unknown string, `buildContextEntries` silently falls back to the latest entry and can build effort history for the wrong branch. Distinguish `undefined` from a string and return an empty context when the string is not present.</comment>

<file context>
@@ -88,3 +88,69 @@ export function collectPiEffortHistory(
+  const byId = new Map(entries.map((entry) => [entry.id, entry]))
+  let leaf: SessionEntryWithParent | undefined
+  if (leafId === null) return []
+  if (leafId) leaf = byId.get(leafId)
+  leaf ??= entries.at(-1)
+  if (!leaf) return []
</file context>
Suggested change
if (leafId) leaf = byId.get(leafId)
leaf ??= entries.at(-1)
if (leafId === undefined) leaf = entries.at(-1)
else leaf = byId.get(leafId)

if (!leaf) return []
const path: SessionEntryWithParent[] = []
let current: SessionEntryWithParent | undefined = leaf
while (current) {
path.push(current)
current = current.parentId ? byId.get(current.parentId) : undefined
}
path.reverse()

let compaction: SessionEntryWithParent | undefined
for (const entry of path) {
if (entry.type === 'compaction') compaction = entry
}
if (!compaction) return path

const compactionIndex = path.findIndex((entry) => entry.id === compaction.id)
if (compactionIndex < 0) return path
const contextEntries: SessionEntryWithParent[] = [compaction]
let foundFirstKept = false
for (let index = 0; index < compactionIndex; index++) {
const entry = path[index]
if (!entry) continue
if (entry.id === compaction.firstKeptEntryId) foundFirstKept = true
if (foundFirstKept) contextEntries.push(entry)
}
contextEntries.push(...path.slice(compactionIndex + 1))
return contextEntries
}
11 changes: 9 additions & 2 deletions packages/pi/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ import type {
import type { ExtensionAPI } from '@earendil-works/pi-coding-agent'

import { registerCommands } from './commands.ts'
import { collectPiEffortHistory } from './effort-history.ts'
import {
buildContextEntries,
collectPiEffortHistory,
resolveSessionLeafId,
} from './effort-history.ts'
import { streamCortexKitAnthropic } from './stream.ts'

async function loginAnthropic(
Expand Down Expand Up @@ -72,7 +76,10 @@ export default function cortexKitPiAnthropicAuth(pi: ExtensionAPI) {
const sessionId = ctx.sessionManager.getSessionId()
if (!sessionId) return
const transitions = collectPiEffortHistory(
ctx.sessionManager.buildContextEntries(),
buildContextEntries(
ctx.sessionManager.getEntries(),
resolveSessionLeafId(ctx.sessionManager),
),
ctx.sessionManager.getBranch(),
)
effortHistoryBySession.delete(sessionId)
Expand Down
106 changes: 105 additions & 1 deletion packages/pi/src/tests/effort-history.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, test } from 'bun:test'
import { collectPiEffortHistory } from '../effort-history.ts'
import {
buildContextEntries,
collectPiEffortHistory,
resolveSessionLeafId,
} from '../effort-history.ts'

const entry = (
id: string,
Expand Down Expand Up @@ -52,3 +56,103 @@ describe('Pi Fable 5.1 effort history', () => {
])
})
})

describe('buildContextEntries', () => {
const contextEntry = (
id: string,
parentId: string | null,
type = 'message',
extra: Record<string, unknown> = {},
) => ({ id, parentId, type, ...extra })

test('uses the latest entry when the host has no leaf method', () => {
const entries = [contextEntry('a', null), contextEntry('b', 'a')]
expect(
buildContextEntries(entries, undefined).map((entry) => entry.id),
).toEqual(['a', 'b'])
})

test('preserves an absent host leaf method as undefined', () => {
const host = { getEntries: () => [] }
expect(resolveSessionLeafId(host)).toBeUndefined()
expect(
buildContextEntries(
[contextEntry('a', null), contextEntry('b', 'a')],
resolveSessionLeafId(host),
).map((entry) => entry.id),
).toEqual(['a', 'b'])
})

test('preserves an explicit null host leaf', () => {
const host = { getLeafId: () => null }
expect(resolveSessionLeafId(host)).toBeNull()
})

test('returns no entries for an explicit null leaf', () => {
const entries = [contextEntry('a', null)]
expect(buildContextEntries(entries, null)).toEqual([])
})

test('follows a leaf path from the root', () => {
const entries = [
contextEntry('a', null),
contextEntry('b', 'a'),
contextEntry('c', 'b'),
contextEntry('other', 'a'),
]
expect(buildContextEntries(entries, 'c').map((entry) => entry.id)).toEqual([
'a',
'b',
'c',
])
})

test('keeps a compaction and entries starting at firstKeptEntryId', () => {
const entries = [
contextEntry('a', null),
contextEntry('kept', 'a'),
contextEntry('drop', 'kept'),
contextEntry('compact', 'drop', 'compaction', {
firstKeptEntryId: 'kept',
}),
contextEntry('after', 'compact'),
]
expect(
buildContextEntries(entries, 'after').map((entry) => entry.id),
).toEqual(['compact', 'kept', 'drop', 'after'])
})

test('returns the full path when there is no compaction', () => {
const entries = [contextEntry('a', null), contextEntry('b', 'a')]
expect(buildContextEntries(entries, 'b').map((entry) => entry.id)).toEqual([
'a',
'b',
])
})

test('uses only the latest compaction on the path', () => {
const entries = [
contextEntry('a', null),
contextEntry('kept1', 'a'),
contextEntry('compact1', 'kept1', 'compaction', {
firstKeptEntryId: 'kept1',
}),
contextEntry('kept2', 'compact1'),
contextEntry('compact2', 'kept2', 'compaction', {
firstKeptEntryId: 'kept2',
}),
contextEntry('after', 'compact2'),
]
expect(
buildContextEntries(entries, 'after').map((entry) => entry.id),
).toEqual(['compact2', 'kept2', 'after'])
})

test('passes branch summaries through unchanged', () => {
const summary = contextEntry('summary', 'a', 'branch_summary', {
summary: 'branch',
})
const entries = [contextEntry('a', null), summary]
expect(buildContextEntries(entries, 'summary')).toEqual(entries)
})
})