refactor(frontend): Move the PanelSection family into @agenta/ui - #5768
refactor(frontend): Move the PanelSection family into @agenta/ui#5768ardaerzin wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change documents package extraction for sessions and agent surfaces. It adds shared panel layout primitives and public exports. It also updates the RichChatInput background token. ChangesSessions package extraction and UI foundation
Composer surface styling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Railway Preview Environment
Updated at 2026-08-09T16:56:33.460Z |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
web/packages/agenta-ui/src/RichChatInput/RichChatInput.tsx (1)
215-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the in-code rationale.
This three-line comment exceeds the repository rule that in-code comments must be at most one short line unless they document a genuinely surprising constraint. Keep one concise explanation here and move the visual design rationale to design documentation if it must be preserved.
As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
docs/design/sessions-packages/plan.md (1)
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the rail layout contract before consumers adopt it.
The plan defines one bordered rail with hairline-separated sections.
web/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsxinstead makesPanelSurfacetransparent, gives each rail section its own rounded border, and adds spacing between sections. Update the plan or implementation so the shared layout contract has one border and spacing model.web/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsx (1)
48-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an explicit keyboard focus state to the shared action class.
PANEL_ACTION_CLASSdefines default and hover colors but nofocus-visiblestate. Add a semantic-token focus ring or text state so a consumer that uses this shared class alone still exposes keyboard focus.As per coding guidelines, “Implement light and dark appearance and interaction states for every added or changed UI element.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a9d51ce-cad4-4d4b-ac41-9d8f93fb32b6
📒 Files selected for processing (5)
docs/design/sessions-packages/plan.mdweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/components/presentational/index.tsweb/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsxweb/packages/agenta-ui/src/components/presentational/layout/index.tsx
f800d4a to
96f9e4b
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
web/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsx (1)
3-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the visual-design rationale comments.
Lines 3-14 and Lines 44-50 describe design history and rationale. Keep one short component-purpose comment in the source. Put longer design rationale in design documentation.
As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Also applies to: 44-50
Source: Coding guidelines
docs/design/sessions-packages/plan.md (1)
20-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude
@agenta/entity-uiin the layering contract.The diagram omits
@agenta/entity-ui, but Lane 4 defines it as a package with a public./agentsubpath. Show it as a branch after@agenta/entities, or explain why it is outside this dependency graph.Proposed diagram update
-@agenta/shared → `@agenta/ui` → `@agenta/entities` → `@agenta/sessions` → `@agenta/sessions-ui` +@agenta/shared → `@agenta/ui` → `@agenta/entities` + ├→ `@agenta/entity-ui` + └→ `@agenta/sessions` → `@agenta/sessions-ui`As per coding guidelines, workspace packages must respect the hierarchy
shared ← ui ← entities ← entity-ui ← playground ← playground-ui.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d4e37fa-105c-462a-84e1-393c406bee59
📒 Files selected for processing (5)
docs/design/sessions-packages/plan.mdweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/components/presentational/index.tsweb/packages/agenta-ui/src/components/presentational/layout/PanelSection.tsxweb/packages/agenta-ui/src/components/presentational/layout/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- web/packages/agenta-ui/src/RichChatInput/RichChatInput.tsx
- web/packages/agenta-ui/src/components/presentational/index.ts
- web/packages/agenta-ui/src/components/presentational/layout/index.tsx
| `@agenta/entities/session` keeps what it owns: zod schema, `listOptions`, the query itself. | ||
| `@agenta/sessions` is new and owns **orchestration**. That boundary is what reconciles this | ||
| with the earlier "no new package for entity state" decision. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'\b(useSessionList|listOptions|waitingSessionIds|excludeOrigin|infiniteQuery)\b' \
web/packages \
--glob '*.ts' \
--glob '*.tsx'Repository: Agenta-AI/agenta
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked files matching plan/session/session packages:\n'
git ls-files | rg -n 'session|packages|design|plan\.md' | sed -n '1,160p'
printf '\nDesign plan location:\n'
fd -a 'plan\.md' . | sed -n '1,120p'
printf '\nSearch key symbols in repository:\n'
rg -n -C 3 \
'\b(useSessionList|listOptions|waitingSessionIds|excludeOrigin|infiniteQuery|`@agenta/entities/session`|`@agenta/sessions`)\b' \
. \
--glob '!node_modules' \
--glob '!dist' \
--glob '!build' \
--glob '!*.lock' | sed -n '1,240p'Repository: Agenta-AI/agenta
Length of output: 20857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant sections of docs/design/sessions-packages/plan.md:\n'
sed -n '1,220p' docs/design/sessions-packages/plan.md | nl -ba
printf '\nAll occurrences of session query/package ownership terms in plan:\n'
rg -n '\b(session query|the query itself|infinite query|session list|listOptions|`@agenta/`(entities/session|sessions))' docs/design/sessions-packages/plan.mdRepository: Agenta-AI/agenta
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant sections of docs/design/sessions-packages/plan.md:\n'
cat -n docs/design/sessions-packages/plan.md | sed -n '1,220p'
printf '\nPackage ownership mentions:\n'
rg -n '\b(session query|the query itself|infinite query|session list|listOptions|`@agenta/`(entities/session|sessions))' docs/design/sessions-packages/plan.md
printf '\nPackage existence/context:\n'
git ls-files | rg -n 'packages/agenta/(packages/)?(`@agenta/`|entities/sessions?|sessions/)' || trueRepository: Agenta-AI/agenta
Length of output: 11743
Clarify which query useSessionList belongs to.
The plan assigns “the query itself” to @agenta/entities/session, but Lane 2 moves state/useSessionList.ts with “the infinite query, cursor handling, the waitingSessionIds pushdown, [and] excludeOrigin semantics” into @agenta/sessions. State whether @agenta/entities/session owns only the low-level fetch/query contract, while @agenta/sessions owns the query hook, paging, filters, groups, and derived state, or adjust the ownership assignments so the package boundary is unambiguous.
[low Effort_and_high_reward]
Source: Coding guidelines
| - `pnpm lint-fix` clean; `pnpm --filter @agenta/oss exec tsc --noEmit` shows no new errors | ||
| (gate on the error-signature diff, not the count — it fluctuates with cache) | ||
| - Package lanes: `pnpm turbo run build --filter=@agenta/<pkg>` and their unit tests | ||
| - Lanes 1, 3, 4: grep the package for `from "antd"` → must be empty |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the scoped ESLint rule for the antd verification gate.
The current grep misses single-quoted imports and subpath imports such as antd/es/.... Use the package-scoped ESLint rule as the gate, and ensure the rule covers all prohibited import patterns.
Proposed verification update
-Lanes 1, 3, 4: grep the package for `from "antd"` → must be empty
+Lanes 1, 3, 4: run the package-scoped ESLint rule that forbids all prohibited `antd` imports; the rule must pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Lanes 1, 3, 4: grep the package for `from "antd"` → must be empty | |
| - Lanes 1, 3, 4: run the package-scoped ESLint rule that forbids all prohibited `antd` imports; the rule must pass |
PanelSurface, PanelScroll, PanelSection and PANEL_ACTION_CLASS graduate from the oss app into @agenta/ui's presentational layer so every surface (and later mobile) shares one section language. The composer fill on RichChatInput rides along. The sessions-packages plan doc records the lane decisions.
96f9e4b to
3ff33cb
Compare
|
Landed in |
Context
Bottom lane of the sessions/agents UX stack (plan:
docs/design/sessions-packages/plan.md, included here). The Home, overview, templates and sessions pages all speak one "section language" (surface, scroll area, section header with a right-aligned action). That family lived in the oss app, so nothing outside it (and later, mobile) could reuse it.Changes
PanelSurface,PanelScroll,PanelSectionandPANEL_ACTION_CLASSmove to@agenta/ui's presentational layout layer. They were already antd-free. Two behaviors are documented in the code and preserved: sticky section headers need an opaque background (rows scroll through rgba fills), and a scroll container withpadding-topcannot host atop-0sticky child, so the sticky element supplies its own top spacing.RichChatInputpicks up the composer fill (colorFillTertiary) so composers read as fields, not outlines.Tests / notes
pnpm turbo run build --filter=@agenta/uipasses;@agenta/osstsc is clean on this lane.