[AGE-4141] feat(frontend): Persist an agent's icon on its workflow artifact - #6922
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (12)
📝 SummarySummary by CodeRabbit
WalkthroughWorkflow agent icons are now stored in workflow artifact tags. The implementation validates icon settings, resolves SVG paths from the catalog, preserves unrelated tags, updates caches, supports optimistic saves, and retains legacy browser values as fallback. ChangesAgent icon persistence
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant IconPicker
participant agentIconAtomFamily
participant queryWorkflows
participant updateWorkflow
participant WorkflowCaches
IconPicker->>agentIconAtomFamily: select icon and color
agentIconAtomFamily->>queryWorkflows: fetch latest workflow tags
agentIconAtomFamily->>updateWorkflow: save merged `@ag.icon` tag
updateWorkflow-->>agentIconAtomFamily: return success or failure
agentIconAtomFamily->>WorkflowCaches: patch artifact caches
Merge Risk: 🟡 Moderate · up to Icon updates can remove unrelated workflow metadata, while cleared or temporarily unavailable icons may render incorrectly. These issues should be fixed before merge. 🚥 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
|
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 000812a1-541f-4b18-a7c5-da0e8a2643aa
📒 Files selected for processing (12)
web/oss/src/components/SharedDrawers/AnnotateDrawer/assets/CreateEvaluator/index.tsxweb/packages/agenta-entities/src/workflow/api/api.tsweb/packages/agenta-entities/src/workflow/index.tsweb/packages/agenta-entities/src/workflow/state/agentIcon.tsweb/packages/agenta-entities/src/workflow/state/evaluatorUtils.tsweb/packages/agenta-entities/src/workflow/state/index.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/__mocks__/agenta-ui-agent-icon.tsweb/packages/agenta-entities/tests/__mocks__/agenta-ui-app-message.tsweb/packages/agenta-entities/tests/unit/agent-icon-record.test.tsweb/packages/agenta-entities/tests/unit/agent-icon.test.tsweb/packages/agenta-entities/vitest.config.ts
💤 Files with no reviewable changes (1)
- web/packages/agenta-entities/tests/unit/agent-icon-record.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| } else { | ||
| tags = get(workflowArtifactQueryAtomFamily(workflowId)).data?.tags | ||
| } | ||
| return readAgentIconTag(tags) ?? readLegacySetting(get(legacyIconMapAtom)?.[workflowId]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '40,220p' web/packages/agenta-entities/src/workflow/state/agentIcon.ts
sed -n '150,210p' web/packages/agenta-entities/tests/unit/agent-icon.test.tsRepository: Agenta-AI/agenta
Length of output: 10204
🏁 Script executed:
sed -n '1,170p' web/packages/agenta-entities/tests/unit/agent-icon.test.ts
printf '\n--- legacy references ---\n'
rg -n -C 5 'legacy|localStorage|agent-icon:1|clear.*icon|icon.*clear' web/packages/agenta-entities/tests web/packages/agenta-entities/src/workflow/state/agentIcon.tsRepository: Agenta-AI/agenta
Length of output: 50372
Persist an explicit clear or migration marker.
When a successful null write removes @ag.icon, agentIconSettingAtomFamily falls back to readLegacySetting. The write removes the legacy entry only from the current browser. Another browser with the old localStorage value can render the stale icon because the artifact has no marker that distinguishes an explicit clear from an unmigrated workflow.
Use the legacy value only when the artifact has no migration marker. Persist that marker when the icon is cleared.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 1937a62b-c240-41ea-8ecb-a8f30ccf356b
📒 Files selected for processing (2)
web/packages/agenta-entities/src/workflow/state/agentIcon.tsweb/packages/agenta-entities/tests/unit/agent-icon.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
… tags.@ag.icon The picker's choice was in localStorage, so no other device or teammate saw it (#6082). It now goes to the artifact as {name, color} — the glyph is resolved by name from the catalog, so no SVG is stored or rendered from data. The seam is unchanged: every surface still reads agentIconAtomFamily, and the picker still writes it. The write is optimistic behind a synchronous overlay (the query observer notifies a tick late, and a colour picked in that tick built on the old icon), saves are serialised per agent, and the merged map is fetched fresh before the PUT because the edit replaces tags. An emptied map is sent as {} — the API drops a null field and would keep the old tags. Also: the payload types said tags were string[] against a nested-map schema; the evaluator drawer was sending Object.keys(tags), which the API rejects; and updateWorkflow skipped the PUT for tags: null.
…e fallback glyph unwrap only substitutes NO_GLYPHS while the catalog promise is pending; a rejection threw out of agentIconAtomFamily and into the nearest error boundary, taking the whole surface with it. Swallow the failure into an empty map instead. The loader already drops a rejected promise, so the picker's own retry is unaffected.
…ls a glyph map a failed load left empty Two review findings. If the by-id query comes back without the workflow (a validation failure, say), merging into nothing would write an icon-only map and wipe every other tag; now the save throws into the rollback path instead. And a catalog load that failed once left the glyph map empty for the session, so configured icons stayed on the fallback even after the picker had loaded the catalog fine; the map is refreshable now and a successful pick refills it.
4ebb4db to
eac62be
Compare
Context
The agent icon picker (#6062) kept its choice in the browser's localStorage. Pick a robot and a colour on your laptop, and your phone, a second browser, and every teammate still saw the default glyph. The choice needs to live with the agent.
Closes #6082.
Changes
The icon now lives on the workflow artifact, under
tags["@ag"].icon. Only the name and the colour travel; the glyph is looked up by name in the generated catalog, so no SVG markup is stored or rendered from data.Stored on the artifact:
{"tags": {"@ag": {"icon": {"name": "robot", "color": "#113955"}}}}Nothing changes for the surfaces. Every chip, row and avatar still reads
agentIconAtomFamily(workflowId), and the picker still writes it. Only the atom behind that seam changed:@ag.iconmerged into them (the edit endpoint replaces the whole map, so every other tag comes back with it), then the PUT goes out. On failure the chip snaps back and a toast says so.{}rather thannull. The API drops a null field and would have kept the old tags.Three type fixes came out of this:
UpdateWorkflowPayload.tags(and its create and evaluator siblings) saidstring[]against a nested-map schema; the evaluator drawer was sendingObject.keys(tags), which the API rejects, and now passes the map through; andupdateWorkflowskipped the PUT whentagswas falsy.No backend or SDK change.
Tagsalready accepts nested maps,PUT /workflows/{id}writes them, and every workflow read returns them.Tests
agent-icon.test.ts: tag read and merge, optimistic write, rollback on a failed save, the null clear, and two picks in flight saving in order.@agenta/entities,@agenta/entity-ui,web/mobileandweb/oss./magainst the EE dev stack: the PUT body carries{name, color}and no SVG, the row inworkflow_artifactsholds the same, and Reset leaves{}.updateWorkflowstill fetches a revision after every metadata PUT.What to QA
/m, open an agent, click its chip, pick an icon and a colour. Reload with localStorage cleared: the icon is still there. Open the same agent in another browser or as another member: same icon.