fix(web): unify session header display labels - #1158
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
None.
Questions
None.
Summary
Review mode: initial
No issues found in the modified lines. Residual risk: I did not run tests in this review environment; the PR adds helper-level coverage for agent labels, catalog display names, Codex fallback formatting, and unknown IDs, but there is no component-level test around the SessionHeader catalog-backed display path.
Testing
Not run (automation).
HAPI Bot
7497512 to
7a498c8
Compare
There was a problem hiding this comment.
Findings
- [Minor] Codex model catalog query still runs when the header renders nothing in Telegram — the new
enabledexpression can be true for active Codex sessions before the existing Telegram early return, so Telegram views can still hit/codex-modelsfor UI that is not rendered. That RPC opens a Codex app-server client and callslistModels, so this adds avoidable runner work. Evidenceweb/src/components/SessionHeader.tsx:117; contextweb/src/components/SessionHeader.tsx:217,cli/src/modules/common/codexModels.ts:87.
Suggested fix:const telegramApp = isTelegramApp() const codexModelsState = useCodexModels({ api, machineId: session.metadata?.machineId ?? null, enabled: !telegramApp && agentFlavor === 'codex' && session.active && session.agentState?.controlledByUser !== true }) // ... if (telegramApp) { return null }
Questions
None.
Summary
Review mode: follow-up after new commits
Found one minor performance regression in the latest diff. Prior HAPI Bot review had no findings; latest PR diff reviewed in full, with the compare diff used only as follow-up context.
Testing
Not run (automation). PR reports bun typecheck and NODE_OPTIONS=--max-old-space-size=8192 bun run test:web.
HAPI Bot
| const codexModelsState = useCodexModels({ | ||
| api, | ||
| machineId: session.metadata?.machineId ?? null, | ||
| enabled: agentFlavor === 'codex' && session.active && session.agentState?.controlledByUser !== true |
There was a problem hiding this comment.
[MINOR] Disable this query when the header is hidden in Telegram. SessionHeader still calls useCodexModels before the existing isTelegramApp() early return, so this can be enabled for active Codex sessions even though the component returns null. The endpoint opens a Codex app-server client and calls listModels, making this avoidable runner work for UI that is not rendered.
Suggested fix:
const telegramApp = isTelegramApp()
const codexModelsState = useCodexModels({
api,
machineId: session.metadata?.machineId ?? null,
enabled: !telegramApp
&& agentFlavor === 'codex'
&& session.active
&& session.agentState?.controlledByUser !== true
})
// ...
if (telegramApp) {
return null
}
7a498c8 to
304f2e4
Compare
There was a problem hiding this comment.
Findings
- [Minor] The Telegram-only Codex model catalog request reported in the prior bot review remains on the current head. See the inline comment at
web/src/components/SessionHeader.tsx:144.
Questions
None.
Summary
Review mode: follow-up after new commits
One previously reported minor performance issue remains. The latest PR diff was reviewed in full; no additional findings met the 80% confidence threshold. Residual testing gap: there is no component test confirming that Telegram rendering suppresses the catalog request.
Testing
Not run (automation; PR content was not executed). PR reports bun typecheck and NODE_OPTIONS=--max-old-space-size=8192 bun run test:web.
HAPI Bot
| const codexModelsState = useCodexModels({ | ||
| api, | ||
| machineId: session.metadata?.machineId ?? null, | ||
| enabled: agentFlavor === 'codex' && session.active && session.agentState?.controlledByUser !== true |
There was a problem hiding this comment.
[MINOR] Gate this query on Telegram visibility. It is enabled before the isTelegramApp() early return at line 288, so an active Codex session in Telegram still requests /machines/:id/codex-models even though SessionHeader renders nothing. That endpoint opens a Codex app-server client and calls listModels, adding avoidable runner work.
Suggested fix:
const telegramApp = isTelegramApp()
const codexModelsState = useCodexModels({
api,
machineId: session.metadata?.machineId ?? null,
enabled: !telegramApp
&& agentFlavor === 'codex'
&& session.active
&& session.agentState?.controlledByUser !== true
})
// ...
if (telegramApp) {
return null
}
There was a problem hiding this comment.
Findings
None.
Questions
None.
Summary
Review mode: follow-up after new commits
The prior Telegram-only header catalog-query issue is resolved at web/src/components/SessionHeader.tsx:144, with regression coverage at web/src/components/SessionHeader.test.tsx:94. The latest PR diff was reviewed in full; no remaining correctness, security, regression, data-loss, performance, or maintainability issue met the 80% confidence threshold. Residual testing gap: no component-level test verifies that a loaded catalog display name is rendered in the header; the label helper is unit-covered.
Testing
Not run (automation; PR content was not executed). GitHub checks were pending at review time.
HAPI Bot
ff14e91 to
8148ce4
Compare
There was a problem hiding this comment.
Findings
None.
Questions
None.
Summary
Review mode: follow-up after new commits
The latest PR diff was reviewed in full. No correctness, security, regression, data-loss, performance, or maintainability issue introduced or directly triggered by the diff met the 80% confidence threshold. Residual testing gap: web/src/components/SessionHeader.test.tsx verifies that Telegram disables the catalog query, but no component-level test verifies that a loaded catalog display name is rendered; the label helper is unit-covered.
Testing
Not run (automation; PR content was not executed). The current GitHub test check failed during bun typecheck at web/src/router.tsx:773 because deliveryMode is missing from a RawSendError; that file is unchanged by this PR, so the failure is not attributed to this diff. Web tests were not reached.
HAPI Bot
8148ce4 to
0b50893
Compare
There was a problem hiding this comment.
Findings
- [Minor] Avoid starting a Codex catalog lookup when its result cannot be rendered — the new query is enabled for every active remote Codex session, but the result is unused when the model metadata preference is off, and
getSessionModelLabelalso discards the catalog display name whensession.modelis empty. The endpoint dispatches a machine RPC that starts a Codex app-server client, so opening a header can perform expensive work with no UI consumer. Evidence:web/src/components/SessionHeader.tsx:171; related context:web/src/lib/sessionModelLabel.ts:43,cli/src/modules/common/codexModels.ts:83.
Suggested fix:const explicitModel = typeof session.model === "string" && session.model.trim() !== "" enabled: !telegramApp && headerMetadata.model && explicitModel && agentFlavor === "codex" && session.active && session.agentState?.controlledByUser !== true
Questions
None.
Summary
Review mode: follow-up after new commits
One Minor performance issue found in the full current diff. The new Telegram guard resolves the prior environment-specific query path, but unused model-catalog lookups remain possible outside Telegram.
Testing
Not run (automation; PR-authored code was not executed). Add a SessionHeader test that disables model metadata or supplies model: null and asserts the catalog query is disabled; retain a component-level case proving an explicit model uses the catalog display name.
HAPI Bot
| const codexModelsState = useCodexModels({ | ||
| api, | ||
| machineId: session.metadata?.machineId ?? null, | ||
| enabled: !telegramApp |
There was a problem hiding this comment.
[MINOR] Avoid an unused Codex catalog RPC
This condition still enables the catalog query when model metadata is disabled or session.model is null/blank. In both cases this component cannot render the response: the preference removes the model row, and getSessionModelLabel returns null without an explicit model. Since the endpoint performs a machine RPC and initializes a Codex app-server client, gate the query on both conditions.
Suggested fix:
const explicitModel = typeof session.model === "string" && session.model.trim() !== ""
enabled: !telegramApp
&& headerMetadata.model
&& explicitModel
&& agentFlavor === "codex"
&& session.active
&& session.agentState?.controlledByUser !== true
There was a problem hiding this comment.
Findings
No findings.
Questions
None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The prior Minor catalog-RPC finding is resolved by requiring both visible model metadata and a nonblank session model before enabling the query. Residual risk is limited to the pending repository test workflow.
Testing
Not run (automation; PR-authored code was not executed). Static review covered the added query guards, catalog display-name resolution, Codex fallback formatting, agent labels, and their unit/component tests. GitHub's test check was still in progress when reviewed.
HAPI Bot
c8e679b to
9323174
Compare
There was a problem hiding this comment.
Findings
No findings.
Questions
None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The prior catalog-RPC finding remains resolved: lookups require a visible, nonblank Codex model row and are disabled in Telegram, inactive/local sessions, and sessions without a usable machine/API target. Residual risk is limited to the pending repository test workflow.
Testing
Not run (automation; PR-authored code was not executed). Static review covered the query guards, catalog display-name resolution, Codex fallback formatting, agent-label mapping, surrounding consumers, and the added unit/component tests.
HAPI Bot
…splay-labels # Conflicts: # web/src/components/SessionHeader.test.tsx
There was a problem hiding this comment.
Findings
No findings.
Questions
None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The new head only merges current main; the four-file PR diff still preserves the prior query guards and label-resolution behavior. Residual risk: the repository test workflow was still running at review time.
Testing
Not run (automation; PR-authored code was not executed). Static review covered the full diff, prior bot review, changes since that review, and surrounding catalog/flavor/query implementations.
HAPI Bot
1 participant
Summary
gpt-*model IDs when the catalog is unavailable, including inactive sessions, while preserving unknown model IDsUnknownProblem
The session header rendered raw metadata identifiers such as
codexandgpt-5.6-sol, while the new-session and settings UIs rendered the same values asCodexandGPT-5.6-Sol. This made the product naming inconsistent, especially on mobile.Testing
bun typecheckNODE_OPTIONS=--max-old-space-size=8192 bun run test:web(165 files, 1396 tests)