feat(atomic) creation of agent generation steps component#7165
feat(atomic) creation of agent generation steps component#7165SimonMilord wants to merge 11 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Atomic (Lit) functional renderer intended to display agent generation steps (search/think/generate) while a generated answer is streaming, along with supporting localization strings, styling for a rolodex-like transition, and unit tests.
Changes:
- Added new i18n keys for agent “search” and “think” generation steps.
- Added generated-answer CSS keyframes/selectors for a rolodex animation.
- Added
renderAgentGenerationStepsfunctional component plus helper functions and unit tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| packages/atomic/src/locales.json | Adds new localized strings for agent generation step labels. |
| packages/atomic/src/components/common/generated-answer/styles/generated-answer.tw.css.ts | Adds keyframes and styling hooks for the generation-steps rolodex animation. |
| packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.ts | Introduces a new functional renderer + step-selection helpers for agent generation steps. |
| packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.spec.ts | Adds unit tests covering rendering and step-selection behavior. |
packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.ts
Outdated
Show resolved
Hide resolved
packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.ts
Show resolved
Hide resolved
packages/atomic/src/components/common/generated-answer/styles/generated-answer.tw.css.ts
Show resolved
Hide resolved
packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.spec.ts
Outdated
Show resolved
Hide resolved
packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.spec.ts
Outdated
Show resolved
Hide resolved
packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.ts
Show resolved
Hide resolved
packages/atomic/src/components/common/generated-answer/render-agent-generation-steps.ts
Show resolved
Hide resolved
16a50f8 to
fc70014
Compare
mmitiche
left a comment
There was a problem hiding this comment.
The current implementation goes beyond the initial scope by introducing unnecessary complexity and implicit requirements.
At this stage, we should focus on a minimal, well-defined component that fulfills its core responsibility and enables fast iteration.
Especially With tight deadlines, simplicity is the key. Additional behaviour can be evaluated and introduced later if there’s a clear need.
| search: 'agent-generation-step-search', | ||
| think: 'agent-generation-step-think', |
There was a problem hiding this comment.
these two are called searching and thinking.
| currentStepKey, | ||
| html` | ||
| <div | ||
| part="is-generating" |
There was a problem hiding this comment.
the name of the part is good. it's not generic.
| * Selection priority: | ||
| * 1) Timeline-based progression (minimum display duration per step for quick updates). | ||
| * 2) Currently active step. | ||
| * 3) Latest completed step by step priority. |
There was a problem hiding this comment.
Where did this requirement come from?
The goal of the component is to inform the end user of the currently active step being performed by the agent and a not a step previously completed by the agent.
|
|
||
| /** | ||
| * Returns the label key for the latest completed step using step priority. | ||
| * Priority order is `answering` > `think` > `search`. |
There was a problem hiding this comment.
Again where did this requirement come from? we never discussed a priority order.
This encodes additional business logic in the component without an explicit decision or validation, and it adds unnecessary complexity to what should be a simple responsibility.
If this component was generated by AI, that's fine, but that's a clear example of why we should always remain in the driving seat, thoughtfully challenging and reviewing everything and validating whether each behaviour is actually required to prevent AI from drifting or over complicated a simple task.
| * Returns the current generation-step localization key. | ||
| * | ||
| * Selection priority: | ||
| * 1) Timeline-based progression (minimum display duration per step for quick updates). |
There was a problem hiding this comment.
We could make the animation run faster, 1500ms seems to be a lot.
and we can start simple by not supporting this minimum display duration. first because we have not yet tried the feature with the real API so far so we don't yet have a concrete idea of how much time the steps will take.
So Basically I found this approach better: Start simple without minimum display duration, make this component reflect the state.
when we will be able to test with the real backend we will try to re-assess and evaluate our best options.
| /** | ||
| * Returns the current generation-step localization key. | ||
| * | ||
| * Selection priority: | ||
| * 1) Timeline-based progression (minimum display duration per step for quick updates). | ||
| * 2) Currently active step. | ||
| * 3) Latest completed step by step priority. | ||
| */ | ||
| export function getCurrentStepKey( | ||
| agentSteps: AgentStep[], | ||
| now = Date.now() | ||
| ): string | undefined { | ||
| const nextStepKey = | ||
| getActiveStepKey(agentSteps) ?? getLatestCompletedStepKey(agentSteps); | ||
|
|
||
| if (!nextStepKey) { | ||
| lastDisplayedStepKey = undefined; | ||
| lastDisplayedStepAt = 0; | ||
| return undefined; | ||
| } | ||
|
|
||
| if (!lastDisplayedStepKey || lastDisplayedStepKey === nextStepKey) { | ||
| if (!lastDisplayedStepKey) { | ||
| lastDisplayedStepKey = nextStepKey; | ||
| lastDisplayedStepAt = now; | ||
| } | ||
| return lastDisplayedStepKey; | ||
| } | ||
|
|
||
| if (now - lastDisplayedStepAt < MIN_STEP_DISPLAY_DURATION_MS) { | ||
| return lastDisplayedStepKey; | ||
| } | ||
|
|
||
| lastDisplayedStepKey = nextStepKey; | ||
| lastDisplayedStepAt = now; | ||
|
|
||
| return lastDisplayedStepKey; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the label key for the currently active step, when present. | ||
| */ | ||
| export function getActiveStepKey(agentSteps: AgentStep[]): string | undefined { | ||
| const activeStep = agentSteps.find((step) => step.status === 'active'); | ||
| return activeStep ? stepLabelKeys[activeStep.name] : undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the label key for the latest completed step using step priority. | ||
| * Priority order is `answering` > `think` > `search`. | ||
| */ | ||
| export function getLatestCompletedStepKey( | ||
| agentSteps: AgentStep[] | ||
| ): string | undefined { | ||
| for (const stepName of [...STEP_NAMES].reverse()) { | ||
| const completedStep = agentSteps.find( | ||
| (step) => step.name === stepName && step.status === 'completed' | ||
| ); | ||
|
|
||
| if (completedStep) { | ||
| return stepLabelKeys[completedStep.name]; | ||
| } | ||
| } | ||
|
|
||
| return undefined; | ||
| } |
There was a problem hiding this comment.
That's way too much logic of what could be an equivalent to:
function getCurrentStep(
agentSteps: GenerationStep[]
): 'searching' | 'thinking' | 'answering' | undefined {
return agentSteps.findLast((step) => step.status === 'active')?.name;
}|
Additionally, this PR should also integrate the new functional component to |
SFINT-6647
IN THIS PR:
Note about the component:
For which label to display, it follows this logic:
Selection priority:
-> for example, if the search step last 400ms, it will continue to display for a minimum of 1500ms (the duration of the rolodex animation)
DEMO:
Screen.Recording.2026-02-25.at.2.53.58.PM.mov