add agent thoughts by wrenj · Pull Request #91 · a2ui-project/composer

wrenj · 2026-07-13T22:21:29Z

https://screencast.googleplex.com/cast/NTQ4OTk3NjE2OTk4ODA5Nnw1MzUwM2MyNS04YQ

github-actions · 2026-07-13T22:22:58Z

⚡ A2UI Composer PR Preview

Your automated preview is successfully live:
👉 Launch PR Preview

Note: This environment will be wiped automatically when the PR is merged or closed.

gemini-code-assist

Code Review

This pull request adds support for streaming and displaying the model's thought process ('thinking' blocks) and introduces the ability to abort active prompt streams. Key changes include UI updates for the thinking blocks, a toggleable stop/send button, and backend integration using AbortController to handle stream cancellation. The review feedback suggests simplifying the AutoScrollDirective by using an input setter instead of ngOnChanges, cleaning up several inline type imports in favor of top-level imports, and using a more robust object property check for AbortError in the standalone client.

gemini-code-assist · 2026-07-13T22:24:07Z

+export class AutoScrollDirective implements OnChanges {
+  @Input('a2uiComposerAutoScroll') autoScroll: string | undefined = '';
+  private readonly el = inject(ElementRef);
+
+  ngOnChanges() {
+    requestAnimationFrame(() => {
+      this.el.nativeElement.scrollTop = this.el.nativeElement.scrollHeight;
+    });
+  }
+}


Using an input setter is more idiomatic in Angular for simple input-triggered side effects. It allows you to remove the OnChanges interface implementation and the ngOnChanges lifecycle hook entirely, simplifying the class structure.

export class AutoScrollDirective { private readonly el = inject(ElementRef); @Input('a2uiComposerAutoScroll') set autoScroll(value: string | undefined) { requestAnimationFrame(() => { this.el.nativeElement.scrollTop = this.el.nativeElement.scrollHeight; }); } }

...but please use the input signal, and not the @Input directive.

gemini-code-assist · 2026-07-13T22:24:08Z

    ];
  }

+  private activeStreamResponse?: import('../llm-client/llm-client').LlmStreamResponse;


Avoid using inline imports for types that can be imported at the top of the file. Please add LlmStreamResponse to the existing imports from ../llm-client/llm-client at the top of the file.

Suggested change

private activeStreamResponse?: import('../llm-client/llm-client').LlmStreamResponse;

private activeStreamResponse?: LlmStreamResponse;

gemini-code-assist · 2026-07-13T22:24:08Z

+    const config: import('@google/genai').GenerateContentConfig = systemInstruction
+      ? {systemInstruction}
+      : {};


Avoid using inline imports for types that can be imported at the top of the file. Please add GenerateContentConfig to the imports from @google/genai at the top of the file.

Suggested change

const config: import('@google/genai').GenerateContentConfig = systemInstruction

? {systemInstruction}

: {};

const config: GenerateContentConfig = systemInstruction

? {systemInstruction}

: {};

gemini-code-assist · 2026-07-13T22:24:08Z

    const responseStream = await ai.models.generateContentStream(params);

-    const buffer: string[] = [];
+    const buffer: import('./llm-client').LlmStreamChunk[] = [];


Avoid using inline imports for types that can be imported at the top of the file. Please add LlmStreamChunk to the imports from ./llm-client at the top of the file.

Suggested change

const buffer: import('./llm-client').LlmStreamChunk[] = [];

const buffer: LlmStreamChunk[] = [];

gemini-code-assist · 2026-07-13T22:24:08Z

+      } catch (err: unknown) {
+        if (err instanceof Error && err.name === 'AbortError') {
+          isDone = true;


The instanceof Error check can sometimes be fragile or fail in certain environments (e.g., cross-realm/iframe environments or specific test runners) when dealing with DOMException or custom abort objects. Using a robust object property check (similar to the one used in chat-coordinator.ts) is safer and more consistent.

Suggested change

} catch (err: unknown) {

if (err instanceof Error && err.name === 'AbortError') {

isDone = true;

} catch (err: unknown) {

if (err && typeof err === 'object' && 'name' in err && err.name === 'AbortError') {

isDone = true;

gemini-code-assist · 2026-07-13T22:24:08Z

+    const contentStream: AsyncIterable<import('./llm-client').LlmStreamChunk> = {
      [Symbol.asyncIterator]() {
        let localBufferIndex = 0;
        return {
-          async next(): Promise<IteratorResult<string>> {
+          async next(): Promise<IteratorResult<import('./llm-client').LlmStreamChunk>> {


Avoid using inline imports for types that can be imported at the top of the file. Please use the top-level LlmStreamChunk import here as well.

Suggested change

const contentStream: AsyncIterable<import('./llm-client').LlmStreamChunk> = {

[Symbol.asyncIterator]() {

let localBufferIndex = 0;

return {

async next(): Promise<IteratorResult<string>> {

async next(): Promise<IteratorResult<import('./llm-client').LlmStreamChunk>> {

const contentStream: AsyncIterable<LlmStreamChunk> = {

[Symbol.asyncIterator]() {

let localBufferIndex = 0;

return {

async next(): Promise<IteratorResult<LlmStreamChunk>> {

jgindin · 2026-07-15T20:26:06Z

+export class AutoScrollDirective implements OnChanges {
+  @Input('a2uiComposerAutoScroll') autoScroll: string | undefined = '';
+  private readonly el = inject(ElementRef);
+
+  ngOnChanges() {
+    requestAnimationFrame(() => {
+      this.el.nativeElement.scrollTop = this.el.nativeElement.scrollHeight;
+    });
+  }
+}


...but please use the input signal, and not the @Input directive.

jgindin · 2026-07-15T20:27:43Z

+
+    .thinking-header {
+      padding: 6px 10px;
+      font-weight: 500;


Pretty sure there's a material token that we should use here: font: var(--mat-sys-label-medium);

Note that this also removes the font-size property above (which is a good thing).

jgindin · 2026-07-15T20:28:59Z

+  /**
+   * Aborts the currently active prompt stream.
+   */
+  protected abortPrompt(): void {


Bit of a nit, but I recommend cancelPrompt instead of abortPropmpt.

jgindin · 2026-07-15T20:29:52Z

+  /**
+   * Aborts the currently active streaming request if there is one.
+   */
+  abortActiveStream(): void {


I recommend changing all usages of abort to cancel

jgindin · 2026-07-15T20:36:10Z

      expect(streamResponse.complete).toBeDefined();

-      const collectedChunks: string[] = [];
+      const collectedChunks: import('./llm-client').LlmStreamChunk[] = [];


Here too (and below), don't use inline imports.

jgindin · 2026-07-15T20:38:44Z

+  readonly thinking?: string;
+}
+
+export interface LlmStreamChunk {


Do we need both the LlmStreamChunk and LlmResponse since they're the same?

If we want to know if something is the complete response or just a chunk, maybe a boolean flag to differentiate between the cases would be simpler?

jgindin · 2026-07-15T20:41:46Z

        notifyListeners();
-      } catch (err) {
+      } catch (err: unknown) {
+        if (err instanceof Error && err.name === 'AbortError') {


Checking the name seems fragile.

At the very least, use a const here and where the Error is thrown from.

thinking

cacce00

wrenj requested review from jgindin and sugoi-yuzuru July 13, 2026 22:21

gemini-code-assist Bot reviewed Jul 13, 2026

View reviewed changes

jgindin reviewed Jul 15, 2026

View reviewed changes

	private activeStreamResponse?: import('../llm-client/llm-client').LlmStreamResponse;
	private activeStreamResponse?: LlmStreamResponse;

	const buffer: import('./llm-client').LlmStreamChunk[] = [];
	const buffer: LlmStreamChunk[] = [];

Uh oh!

Conversation

wrenj commented Jul 13, 2026

Uh oh!

github-actions Bot commented Jul 13, 2026

⚡ A2UI Composer PR Preview

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants