lisa/feat-ai-diagnostics#266
Conversation
gusfcarvalho
left a comment
There was a problem hiding this comment.
Findings inline — all non-blocking nits. CI is green (test + type-check), formatting is clean, and the prior self-review findings are genuinely addressed. Nice separation (composable/partials/view), fail-closed error handling, correct camelcaseStopPaths label-key preservation, and a guarded pagination request. Three small items: a redundant double refreshAll on first load, a notifications-health deep link whose hash doesn't open the panel, and a no-op data.cells.labelSets stop-path.
gusfcarvalho
left a comment
There was a problem hiding this comment.
Thanks — the three prior nits are all addressed in 5ea03f5 (cells.labelSets stop-path removed; notifications hash dropped + link text corrected; double refreshAll guarded with hasCompletedInitialConfigLoad). type-check is clean. One new blocking UX issue found in manual testing: the run-detail drawer is too narrow (pinned at ~320px by the volt theme's p-right:w-80), so the Cells table is cut off. Concrete fix inline — needs an !important width override on the drawer instance.
gusfcarvalho
left a comment
There was a problem hiding this comment.
LGTM. All review findings addressed across 5ea03f5/564e423/80f024f: the 3 original nits (double refreshAll guard, notifications link/hash, no-op stop-path) plus the run-detail drawer width blocker — now w-screen! max-w-6xl!, which correctly overrides the volt p-right:w-80 specificity so the Cells table is usable. CI green (test + type-check), no unresolved threads. Nice work.
|
PR approved. Marking as ready for e2e. |
automated implementation by lisa.