[dolho]

Summary

usePortalAgentPage has always returned error, loaded and reload. Both consumers ignored a different half of them, and the two failures are mirror images:

	failed FIRST load	failed REFRESH
PortalAgentDetails (before)	rendered an empty agent	—
PortalAgentBand (before)	skeleton, forever	InlineError ✓
both (now)	LoadFailed + retry	InlineError beside the data ✓

The reported half

PortalAgentDetails destructured { header, capabilities } only, so a failed /page fell through to the computed fallbacks: identity degraded to the slug, the description vanished, and "This agent hasn't published anything it can do yet" appeared — a positive claim about the agent on no evidence, indistinguishable from a real answer. "Your chats" kept working (a slice of the shell's thread list, not /page), which made the rest read as loaded rather than broken.

The half the issue attributes to the band

The issue cites the band as the surface that already follows the ent#253 rule. It follows half of it: InlineError is gated on loaded, so a failed first load leaves three placeholders pulsing forever with no error and no retry. There are exactly two consumers of this fetch, and fixing one of two is how this class keeps coming back — so both arms are wired here.

Scope of the failure arm

It replaces only the regions /page feeds and deliberately leaves the chat list alone: one wrapper around the whole body would hide a list that is still perfectly good. Every arm gates on the verdict (loaded), never on loading (#2540/#1927).

Tests

Two ent#523 guards pinned the band's literal v-if="!loaded". The properties they protect — skeleton shaped like the row it precedes, gated on the verdict and never on loading — are unchanged, so they are re-pointed at the narrowed gate rather than dropped.

New portalAgentPageFailure.spec.js pins both consumers, both directions, and that the chat list stays out of it. Frontend suite: 2257 passed (102 files); loading-gate ratchet green.

Why now

ent#547 widened it: the body is mounted once per participant in a room, so N independent chances to fail silently — including for a participant off the viewer's roster, where _require_roster answers a uniform 404 by design.

Fixes #2597

🤖 Generated with Claude Code

https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ

[dolho]

/review — PR #2602 (self-review)

Branch: fix/2597-info-body-load-failure → dev · Files: 5
Scope: CLEAN, with one deliberate widening — the issue is written about PortalAgentDetails, and the fix also covers PortalAgentBand. Justification under [I1].

Critical

None.

Findings from this pass — fixed here

[F1] My own failure arm reproduced the bug it was fixing, one component over (Confidence: 9/10)
src/frontend/src/components/portal/PortalAgentBand.vue

The first cut put LoadFailed v-else-if="!loaded" inside <div class="flex flex-wrap items-center gap-x-6 gap-y-2">. Two things wrong, and the second is the real one:

1. Layout. LoadFailed renders a centred icon column at py-6; the band is py-2.5. As a flex item in a row of stat figures it reads wrong, and it roughly triples the band's height on failure, shifting the whole conversation column down — contract principle 4. Its own sibling, the failed-refresh InlineError, sits outside that row. One failure, two languages, two places.

2. The arm had to be a sibling of the row, not a branch inside it. The row's v-else renders stats, which the composable falls back to as { total_executions: 0, timeline: [] }. Zeros are a claim about performance where "no data" is a claim about nothing — so a failure banner sitting above a still-rendering stat row would have been the exact "empty agent" defect this issue is about, reproduced in the component I was adding the fix to.

Both faces are now InlineError, and the failure arm is a sibling of the whole row. Pinned by a guard that asserts the failure arm is ordered before the row rather than nested in it.

[F2] My guard named a primitive instead of a property (Confidence: 8/10)
portalAgentPageFailure.spec.js

expect(band).toMatch(/<LoadFailed[\s\S]{0,240}@retry="reload"/) — so replacing LoadFailed with InlineError turned it red for no behavioural reason. That is precisely the mistake I flagged in the ent#534 guard in #2601 and in the ent#523 guards below, made in a test I wrote in the same session. It now pins the property: an arm gated on error && !loaded that offers a retry, plus a "one failure language" guard.

Consequently the two ent#523 guards in portalAgentPageUx.spec.js go back untouched — v-if="!loaded" is unchanged again, so editing them was collateral of a design I have since replaced. Worth noting as a general lesson: I edited two other people's guards to accommodate a design that did not survive my own review pass.

Informational

[I1] The band fix is outside the issue's stated scope, deliberately (Confidence: 8/10)

#2597 is written about the Info body and cites the band as the surface that already follows the ent#253 rule. It follows half of it: InlineError v-if="error && loaded" covers a failed refresh, and a failed first load left three placeholders pulsing forever with no error and no retry.

There are exactly two consumers of usePortalAgentPage (grepped). Fixing one of two is the incomplete-fix class the repo keeps re-learning (/review 4.14), so both arms are wired. A reviewer who wants the band split out has a fair case; the counter-argument is that the two failures are the same rule and share a composable, and a follow-up issue for the other half of a two-line rule is overhead.

[I2] The panel header still degrades on a failed load — and I am not fixing it (Confidence: 8/10)

The issue lists "identity falls back to the slug, description blank" among the symptoms. I fixed the description (it is gated on header?.description, so it simply does not render) and the capabilities claim, and left the header. Checked rather than assumed:

• healthLabel falls back to 'Status unknown' (PortalAgentDetails.vue:243) — honest;
• availability is computed from props.agent, the roster payload — a different fetch — with header.value?.owner only as an owner fallback, so it stays valid;
• the label degrades to the slug via agentDisplayName.

None of those is a false claim; the slug is a degraded label, not an assertion. The only thing that asserted something untrue was "hasn't published anything it can do yet", which is fixed. Stating this explicitly so the remaining degradation is a recorded decision rather than an oversight.

Clean (verified)

• error/loaded/reload were already exposed by the composable — the whole fix is consumer-side, pinned by a guard so a future change there cannot silently make these guards test nothing.
• The chat list is deliberately outside the failure arm, pinned by a test: it is a slice of the shell's thread list, and one wrapper around the body would hide a list that is still perfectly good.
• Every arm gates on the verdict, never on loading (#2540/#1927); loadingGateRatchet green; raw-colour scan clean on the touched files; semantic status tokens only.
• Frontend suite: 2258 passed (102 files).

Summary

Critical: 0 · Findings fixed in-pass: 2 · Informational: 2 · Scope: one stated widening.

Cannot self-approve; posting as a review comment. [F1] is the substantive one — it was a real defect in my own fix, caught by asking where the element actually renders rather than whether the test passed.

[dolho]

/review — re-review of the fix commits (PR #2602)

Scope: the two commits added since the first review, not the original diff. Verified: full frontend suite 2259 passed (102 files) at the new head.

Re-reviewing my own fix found a defect the fix itself introduced — the useful outcome of this pass.

Critical

None.

Finding — fixed in this pass

[F3] The previous fix over-corrected and removed a recovery affordance (Confidence: 9/10)
PortalAgentBand.vue

The last commit moved the failure out of the stat row and replaced the whole row with it. Re-reading where the elements actually render rather than whether the tests passed:

<div class="flex flex-wrap items-center …">     ← the row I was replacing
  <template v-if="!loaded"> skeleton </template>
  <template v-else> …stats… </template>
  <div class="flex-1"></div>
  <select v-model="timeWindow" …>              ← a SIBLING of the chain, not part of it
</div>

So replacing the row took the window <select> with the numbers. Changing the window is a second way out of a failure — watch(timeWindow, () => load()) re-fetches — alongside the retry button, so a failed 30d load could no longer be answered by trying 7d.

The objection that drove the previous commit was real but specific to the primitive: LoadFailed's centred py-6 column in a py-2.5 band is shaped wrong for a row of stat figures and a layout shift of the conversation column. It was never an objection to the placement. I generalised it one step too far.

The failure is now the first arm of the existing chain, inside the row: InlineError is row-shaped so it costs no layout, the selector renders in every state, and it is still an arm rather than a banner — the v-else branch renders stats, which falls back to { total_executions: 0 }, and zeros are a claim about performance where "no data" is a claim about nothing.

[F4] I had now edited two other people's guards twice for one change (Confidence: 8/10)
portalAgentPageUx.spec.js

The ent#523 guards pin v-if="!loaded". My first design made it !loaded && !error; my second made it v-else-if="!loaded"; each time I edited those two tests to match. Editing someone else's guard twice for one change is the signal that it is pinned too tightly, not that it keeps being wrong.

They now match v-(?:else-)?if="!loaded" — position-independent — so they keep asserting the two properties they exist for (the skeleton's shape; never gating on loading) and stop failing whenever an arm is added above them. Both negative assertions are untouched.

Informational

[I3] Three structural iterations on a two-line rule (Confidence: n/a)

Inside-the-row (LoadFailed) → sibling-of-the-row (InlineError) → arm-inside-the-row (InlineError). The middle step was wrong and the tests passed at every step, because at each step I had pinned the shape I had just written. The guard that finally caught it is the new one — "the selector stays reachable while the failure shows" — which asserts a user-visible consequence rather than a structure. Recorded because it is the transferable lesson from this PR, not because the end state is in doubt.

Clean (re-verified at the new head)

• Every state of the three-arm chain: error && !loaded → InlineError; !loaded → skeleton; otherwise → stats, with the failed-refresh InlineError still below and gated on loaded. No state renders zeros, and none renders both a skeleton and a failure.
• The window selector renders in all three, pinned by a guard that also checks no v-if/v-else opens between the last arm and the <select>.
• LoadFailed is fully gone from the band — import removed, and the "one failure language" guard reads a comment-stripped view so the reference in the explanatory comment cannot make it vacuous.
• PortalAgentDetails is unchanged by these two commits and keeps LoadFailed, which is right there: it sits in a vertical space-y-6 column and replaces a section body, exactly as the Reports section below it already does.
• Suite 2259 passed; loadingGateRatchet green.

Summary

Critical: 0 · Fixed in this pass: 2 · Informational: 1.

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[vybe]

merge-train: ejected — structural conflict with #2604. The compact-header change removed the flex-1 … p-4 space-y-6 wrapper in PortalAgentDetails.vue that this PR inserts its <InlineError> into (two hunks, ~L44–62 and ~L64–173). Intent is compatible but the re-apply is yours. Also: portalAgentPageFailure.spec.js:43 asserts the literal 'reload: load', which #2604 rewrote to const reload = () => load({ force: true }) — that assertion goes red the moment you rebase. Please also change "Related to #2597" to "Fixes #2597". Rides the next train once green.

[vybe]

merge-train (2026-09-09): not on this train — src/frontend/src/components/portal/PortalAgentDetails.vue conflicts with dev, which restructured the panel (the wrapping div.flex-1 was removed and the sections dedented, #2608) after this branch forked. Where the new InlineError sits in the restructured layout is a placement call for you rather than a mechanical re-apply. Merge origin/dev, re-place the block, and it rides the next train.

[dolho]

dev merged in 533eb678; body switched to Fixes #2597.

The #2604 conflict, resolved as you described. Took dev's structure and re-applied this PR's two additions into it:

• ent#547 removed the flex-1 … p-4 space-y-6 wrapper the <InlineError> refresh banner was inserted into, so it is now a direct child of the panel body at dev's indentation. The ent#253 rule it implements is unchanged — a failed refresh keeps everything on screen and says so beside it.
• The LoadFailed arm still comes first in "What it can do", ahead of the empty arm. That ordering is the whole defect: "hasn't published anything" is a positive claim a failed fetch supplies no evidence for.

Two guards were pinning shapes dev has since changed — you called the first, and re-running the suite found its twin:

• portalAgentPageFailure.spec.js:43 asserted the literal 'reload: load'; feat(workspace): the compact header — Info as a rail tab, one paperclip, voice at the composer (abilityai/trinity-enterprise#547, #2580) #2604 rewrote it to const reload = () => load({ force: true }). What the consumers need is that the composable returns something called reload, not that its definition has a particular spelling, so the assertion now says that.
• "keeps the time-window selector reachable while the failure shows" would have gone red next: ent#547 deleted that selector (window fixed at 7 days), so the test asserted a control that no longer exists. Its structural half survives — the failure is an ARM INSIDE the row, so anything else the row holds is a sibling of the chain and cannot be swallowed — and it now pins the selector's absence explicitly rather than passing silently on a missing element.

Frontend suite on the merged branch: 107 files, 2397 tests, all passing.

[vybe]

merge-train (2026-09-09): validated at lane B — /validate-pr + /review clean, touched unit tests and ratchets pass locally against the current dev tip. Below the train floor, so merged individually.