Conversation
There was a problem hiding this comment.
Findings
- [Major] Pi auto-titling overwrites manually chosen session names —
cli/src/pi/extensionUiHandler.ts:142. Details and suggested fix in the attached inline comment.
Summary
Review mode: initial
One regression found in the full four-file diff: generated titles now replace the manual-name field. Keep generated titles in summary and correct the Pi importer’s generated-name assignment.
Testing
Not run (automation). Inspected added tests and existing title/import coverage. Missing regression coverage for preserving manual names through setTitle and displaying updated titles after Pi import. GitHub test checks were pending when inspected.
HAPI Bot
|
|
||
| constructor(private readonly options: PiExtensionUiHandlerOptions) { | ||
| this.syncTitle = createNativeSessionTitleMetadataSync(options.session); | ||
| this.syncTitle = createNativeSessionTitleMetadataSync(options.session, { target: 'name' }); |
There was a problem hiding this comment.
[Major] Preserve manually chosen titles during Pi auto-titling
With a session renamed through the web (metadata.name = "My custom HAPI title"), the next different hapi_change_title / extension setTitle now replaces that saved name. The bundled tool is automatic (cli/src/pi/titleExtension.ts:29), and the new name-target updater overwrites any existing name (cli/src/agent/nativeSessionTitle.ts:58–61). Previously it only updated the summary, preserving the manual title. This violates the precedence explicitly maintained by hub/src/sync/sessionCache.ts:876 and covered by hub/src/sync/sessionCache.titleSummary.test.ts:14.
Keep extension-generated titles in the summary slot; fix the generated-name source in hub/src/web/routes/piSessions.ts:89 so imported transcript titles populate summary instead of claiming name. Preserve existing explicit names there. Suggested fix for this call:
this.syncTitle = createNativeSessionTitleMetadataSync(options.session);Add a regression test starting with a custom metadata.name, delivering a different setTitle, and asserting that the name survives while the summary changes; separately test that an imported session without a manual name displays the new generated title.
fafc709 to
6203d0d
Compare
|
Thanks for the catch — you are right, and the fix now goes the other way. Updated approach (6203d0d): keep the Pi extension
So: manual names survive every auto-title path, and import titles no longer shadow agent-set titles in the web list. |
There was a problem hiding this comment.
Findings
-
[Minor] Imported native titles disappear when user messages exist —
hub/src/web/routes/piSessions.ts:89. Removingnameexposes the last-message-first summary: a native title “Fix login” becomes “thanks” after import. Preserve the transcript title in the summary.Suggested fix (summary selection at line 81):
const summaryText = transcript.title ?? transcript.lastUserMessage
Summary
Review mode: follow-up after new commits
Reviewed the full latest three-file diff. The previous manual-name overwrite is resolved; one import-title regression remains.
Testing
Not run (automation). Inspected added tests and surrounding title/import handling; CI test checks were pending. Add coverage for importing a named native session with a different last user message, and for displaying a subsequent extension title after import.
HAPI Bot
| host: typeof existing.host === 'string' ? existing.host : (machine.metadata?.host ?? machine.id), | ||
| os: typeof existing.os === 'string' ? existing.os : (machine.metadata?.platform ?? process.platform), | ||
| name: typeof existing.name === 'string' ? existing.name : transcript.title, | ||
| name: typeof existing.name === 'string' ? existing.name : undefined, |
There was a problem hiding this comment.
[Minor] Preserve the native transcript title in the summary
Removing the generated name makes summary the displayed title, but buildPiMetadata still chooses transcript.lastUserMessage before transcript.title (line 81). For a native session named “Fix login” whose last user message is “thanks”, a fresh import now displays “thanks”; previously it displayed “Fix login”. The native name is not retained anywhere in the imported metadata.
transcript.title includes explicit Pi session_info.name values (cli/src/modules/common/piSessions.ts:396, 420), not just generated first-message titles. Preserve that title in the summary while retaining this change to the manual-name slot:
const summaryText = transcript.title ?? transcript.lastUserMessageAdd an import regression with a native title and a different last user message; assert metadata.name is absent and getSessionTitle returns the native title. The new test at hub/src/web/routes/piSessions.test.ts:229 currently asserts the message takes precedence and misses this regression.
6203d0d to
e0d5a8a
Compare
|
Good catch on the last-user-message precedence (e0d5a8a). Now |
There was a problem hiding this comment.
Findings
No reportable issues found in the full latest PR diff.
Summary
Review mode: follow-up after new commits
The prior native-title precedence finding is resolved at hub/src/web/routes/piSessions.ts:81. Remaining testing gap: the added tests check import metadata and extension updates separately, without exercising import → extension title update → displayed title together.
Testing
Not run (automation). Inspected the added tests and existing manual-name reconciliation and web title fallback coverage. CI test checks were pending when inspected. Suggested regression: import a session, apply setTitle, and assert the web title changes while a manual name still takes precedence.
HAPI Bot
Pi sessions auto-titling split was wrong: metadata.name is the manually chosen slot (web rename, hub renameSession) that the web title helper (getSessionTitle) prefers, while auto/agent titles belong in metadata.summary.text. The Pi import flow wrote the transcript title into metadata.name, so an agent-set hapi_set_title (which stays in summary) never surfaced in the session list. Fix the source instead of overwriting names from the extension side: - hub/src/web/routes/piSessions.ts: buildPiMetadata no longer falls back to transcript.title for metadata.name; existing manual names are kept, and the title rides in the summary fallback that already preferred lastUserMessage. - hub: regression tests for the import (no manual name -> name stays empty, summary carries lastUserMessage; no user messages at all -> summary falls back to the transcript title). - cli: extensionUiHandler test asserts setTitle keeps a manual metadata.name and lands the agent title in the summary slot, matching sessionCache.ts 'a manually chosen name must continue to win'.
e0d5a8a to
2317b33
Compare
|
Also closed the remaining combination gap (2317b33): a hub test now covers import → later extension-style |
There was a problem hiding this comment.
Findings
No reportable issues found in the full latest PR diff.
Summary
Review mode: follow-up after new commits
Residual testing gap: hub/src/web/routes/piSessions.test.ts:273 simulates the title update with a direct store write; the complete import → extension setTitle → displayed title path remains untested.
Testing
Not run (automation). Inspected added tests, existing manual-name reconciliation coverage, and web title precedence tests. Suggested integration regression: import, apply setTitle, assert the displayed title, then rename manually and verify subsequent agent titles preserve that name. CI test check was pending when inspected.
HAPI Bot
|
Behavior before/after this change (for review convenience) The whole fix is about who owns Field placement on import
Scenario comparison
Unchanged
One caveat (existing data)The change only affects newly imported sessions. Sessions already imported before this change keep the old auto title in |
What
Pi session auto-titling wrote the transcript title into
metadata.name— the slot the web title helper (getSessionTitle) prefers and the one hubrenameSessionuses for manually chosen names. So an agent-set title (thehapi_set_titleextension writesmetadata.summary.text) never surfaced in the web session list; the imported first-message title kept winning.The hub already documents its intended split (
sessionCache.ts): "Keep the generated/native title separate frommetadata.name. A manually chosen name must continue to win in the Web title helper, while the summary remains available as the agent-authored fallback."Fix (fix the source, not the consumer)
hub/src/web/routes/piSessions.ts:buildPiMetadatano longer claimsmetadata.namefromtranscript.title. An existing manualnameis preserved; the imported title rides in the existingsummaryfallback (which already preferslastUserMessage, thentitle).setTitlepath stays onmetadata.summary.text— it must not overwrite a manually chosen name.Tests
hub/src/web/routes/piSessions.test.ts(new, x2): imported transcript title stays out ofmetadata.namewhen the session has no manual name (summary carrieslastUserMessage); and a session with no user messages falls back to the transcript title in summary.cli/src/pi/extensionUiHandler.test.ts: regression — with a manualmetadata.nameset, asetTitledelivers the title to the summary slot and the name survives unchanged.bun run typecheckgreen across all packages (relay/hub/cli/web); cli + hub suites pass.Before/after
metadata.nameAI disclosure: this change was implemented with assistance from the AI model
deepseek/deepseek-v4-flash.