kaxil commented on code in PR #63081:
URL: https://github.com/apache/airflow/pull/63081#discussion_r2908755724


##########
providers/common/ai/src/airflow/providers/common/ai/utils/hitl_review.py:
##########
@@ -0,0 +1,170 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Shared data models, exceptions, and XCom key constants for HITL Review.
+
+Used by both the API-server-side plugin (``plugins.hitl_review``) and the
+worker-side operator mixin (``mixins.hitl_review``).  Depends only on
+``pydantic`` and the standard library.
+
+**Storage**: all session state is persisted as XCom entries on the running
+task instance.  See the *XCom key constants* below for the key naming scheme.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime, timezone
+from enum import Enum
+from typing import Any, Literal
+
+from pydantic import BaseModel, Field
+
+HumanActionType = Literal["approve", "reject", "changes_requested"]
+
+"""
+These xcom keys are reserved for agentic operator with HITL feedback loop.
+"""
+
+_XCOM_PREFIX = "airflow_hitl_review_"
+
+XCOM_AGENT_SESSION = f"{_XCOM_PREFIX}agent_session"
+"""Session metadata written by the **worker**.
+
+Value: ``{"status": "...", "iteration": N, "max_iterations": M,
+"prompt": "...", "current_output": "..."}``.
+"""
+
+XCOM_HUMAN_ACTION = f"{_XCOM_PREFIX}human_action"
+"""Human action command written by the **plugin**.
+
+Value: ``{"action": "approve"|"reject"|"changes_requested",
+"feedback": "...", "iteration": N}``.
+"""
+
+XCOM_AGENT_OUTPUT_PREFIX = f"{_XCOM_PREFIX}agent_output_"
+"""Per-iteration AI output (append-only, written by worker).
+
+Actual key: ``airflow_hitl_review_agent_output_1``, ``_2``, ...
+"""
+
+XCOM_HUMAN_FEEDBACK_PREFIX = f"{_XCOM_PREFIX}human_feedback_"
+"""Per-iteration human feedback (append-only, written by plugin).
+
+Actual key: ``airflow_hitl_review_human_feedback_1``, ``_2``, ...
+"""
+
+
+class SessionStatus(str, Enum):
+    """Lifecycle states of a HITL review session."""
+
+    PENDING_REVIEW = "pending_review"
+    CHANGES_REQUESTED = "changes_requested"
+    APPROVED = "approved"
+    REJECTED = "rejected"
+    MAX_ITERATIONS_EXCEEDED = "max_iterations_exceeded"
+    TIMEOUT_EXCEEDED = "timeout_exceeded"
+
+
+class ConversationEntry(BaseModel):
+    """Single turn in the feedback conversation."""
+
+    role: Literal["assistant", "human"]
+    content: str
+    iteration: int
+    timestamp: datetime = Field(default_factory=lambda: 
datetime.now(timezone.utc))
+
+
+class AgentSessionData(BaseModel):
+    """
+    Session metadata stored in the ``airflow_hitl_review_agent_session`` XCom.
+
+    Written by the **worker** only.
+    """
+
+    status: SessionStatus = SessionStatus.PENDING_REVIEW
+    iteration: int = 1
+    max_iterations: int = 5
+    prompt: str = ""
+    current_output: str = ""
+
+
+class HumanActionData(BaseModel):
+    """
+    Human action payload stored in the ``airflow_hitl_review_human_action`` 
XCom.
+
+    Written by the **plugin** only. Invalid ``action`` values (e.g. typos like
+    "approved") fail validation at parse time instead of causing the worker to
+    loop indefinitely.
+    """
+
+    action: HumanActionType
+    feedback: str = ""
+    iteration: int = 1
+
+
+class HumanFeedbackRequest(BaseModel):
+    """Payload for the ``POST .../feedback`` endpoint."""
+
+    feedback: str
+
+
+class HITLReviewResponse(BaseModel):
+    """API response for a HITL review session (combined from multiple XCom 
entries)."""
+
+    dag_id: str
+    run_id: str
+    task_id: str
+    status: str

Review Comment:
   Medium: `status: str` should be `status: SessionStatus` for consistency with 
the TS side (which uses the `SessionStatus` union type) and to get Pydantic 
validation.
   
   The `from_xcom` method at line 163 passes `session.status.value` (a string) 
— if you change this field to `SessionStatus`, you can pass `session.status` 
directly since `SessionStatus` is a `str` enum and Pydantic will accept either 
the enum member or the raw string value.



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/package.json:
##########
@@ -0,0 +1,48 @@
+{
+  "name": "hitl-review",
+  "packageManager": "[email protected]",
+  "private": true,
+  "version": "0.0.0",
+  "engines": {
+    "node": ">=22"
+  },
+  "type": "module",
+  "main": "./dist/main.js",
+  "module": "./dist/main.js",
+  "types": "./dist/main.d.ts",
+  "exports": {
+    ".": {
+      "import": "./dist/main.js",
+      "types": "./dist/main.d.ts"
+    }
+  },
+  "files": [
+    "dist"
+  ],
+  "scripts": {
+    "dev": "vite --port 5174 --strictPort",
+    "build": "vite build",
+    "build:types": "tsc --p tsconfig.lib.json",
+    "build:lib": "vite build",
+    "lint": "tsc --p tsconfig.app.json --noEmit",
+    "preview": "vite preview"
+  },
+  "dependencies": {
+    "@chakra-ui/react": "^3.34.0",
+    "@emotion/react": "^11.14.0",
+    "next-themes": "^0.4.6",

Review Comment:
   Low: `next-themes` is a Next.js-specific package and doesn't appear to be 
imported anywhere in the source. This adds dead weight to `node_modules` and 
`pnpm-lock.yaml`. Safe to remove.



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/hooks/useSession.ts:
##########
@@ -0,0 +1,146 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useCallback, useEffect, useRef, useState } from "react";
+
+import { ApiError, createApi } from "src/api";
+import type { SessionResponse } from "src/types/feedback";
+
+const POLL_INTERVAL_MS = 3000;
+
+interface UseSessionReturn {
+  session: SessionResponse | null;
+  error: string | null;
+  loading: boolean;
+  /** Whether the task is still running (undefined until first API response). 
*/
+  taskActive: boolean | undefined;
+  sendFeedback: (text: string) => Promise<void>;
+  approve: () => Promise<void>;
+  reject: () => Promise<void>;
+  /** Refetch session (e.g. when polling from NoSession). */
+  refetch: () => Promise<void>;
+}
+
+export function useSession(
+  dagId: string,
+  runId: string,
+  taskId: string,
+  mapIndex: number,
+): UseSessionReturn {
+  const [session, setSession] = useState<SessionResponse | null>(null);
+  const [error, setError] = useState<string | null>(null);
+  const [loading, setLoading] = useState(true);
+  const [taskActive, setTaskActive] = useState<boolean | undefined>(undefined);
+  const timerRef = useRef<ReturnType<typeof setInterval> | null>(null);
+  const apiRef = useRef(createApi(dagId, runId, taskId, mapIndex));
+
+  const isTerminal = useCallback((s: SessionResponse) => {
+    return (
+      s.status === "approved" ||
+      s.status === "rejected" ||
+      s.status === "max_iterations_exceeded" ||
+      s.status === "timeout_exceeded" ||
+      s.task_completed
+    );
+  }, []);
+
+  const stopPolling = useCallback(() => {
+    if (timerRef.current) {
+      clearInterval(timerRef.current);
+      timerRef.current = null;
+    }
+  }, []);
+
+  const fetchSession = useCallback(async () => {
+    try {
+      const data = await apiRef.current.fetchSession();
+      setSession(data);
+      setError(null);
+      setTaskActive(true);
+      if (isTerminal(data)) stopPolling();
+    } catch (err) {
+      if (err instanceof ApiError) {
+        setError(err.message);
+        if (err.taskActive !== undefined) {
+          setTaskActive(err.taskActive);
+          if (!err.taskActive) stopPolling();
+        }
+      } else {
+        setError(err instanceof Error ? err.message : String(err));
+      }
+    } finally {
+      setLoading(false);
+    }
+  }, [isTerminal, stopPolling]);
+
+  useEffect(() => {
+    void fetchSession();
+    timerRef.current = setInterval(() => void fetchSession(), 
POLL_INTERVAL_MS);
+    return stopPolling;
+  }, [fetchSession, stopPolling]);
+
+  const sendFeedback = useCallback(
+    async (text: string) => {
+      setSession((prev) => {
+        if (!prev) return prev;
+        return {
+          ...prev,
+          status: "changes_requested" as const,
+          conversation: [
+            ...prev.conversation,
+            {
+              role: "human" as const,
+              content: text,
+              iteration: prev.iteration,
+            },
+          ],
+        };
+      });
+      try {
+        const data = await apiRef.current.submitFeedback(text);
+        setSession(data);
+      } catch (err) {

Review Comment:
   Medium: If the API call fails here, `setError` is called but the 
optimistically-added message (from the `setSession` call at line 100) stays in 
the conversation. The user sees their message in the chat even though it wasn't 
persisted.
   
   Either save the previous session state before the optimistic update and roll 
it back in the catch block:
   ```ts
   const prevSession = session; // capture before optimistic update
   // ... optimistic setSession ...
   try { ... } catch (err) {
     setSession(prevSession); // roll back
     setError(...);
   }
   ```
   Or accept this UX tradeoff — just calling it out.



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/vite.config.ts:
##########
@@ -0,0 +1,83 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import react from "@vitejs/plugin-react-swc";
+import { resolve } from "node:path";
+import cssInjectedByJsPlugin from "vite-plugin-css-injected-by-js";
+import dts from "vite-plugin-dts";
+import { defineConfig } from "vite";
+
+export default defineConfig(({ command }) => {
+  const isLibraryBuild = command === "build";
+
+  return {
+    base: "./",
+    build: isLibraryBuild
+      ? {
+          chunkSizeWarningLimit: 1600,
+          lib: {
+            entry: resolve("src", "main.tsx"),
+            fileName: "main",
+            formats: ["umd"],
+            name: "AirflowPlugin",
+          },
+          rollupOptions: {
+            external: ["react", "react-dom", "react/jsx-runtime"],

Review Comment:
   Medium: React/ReactDOM/jsx-runtime are correctly externalized here, but 
`@chakra-ui/react` and `@emotion/react` are not — they get bundled into the UMD 
output. Since the Airflow host app already ships Chakra 3 and Emotion, this 
means:
   
   - A second copy of Chakra+Emotion (~200KB+ gzipped) ships in the plugin 
bundle
   - Risk of version mismatch between host's Chakra 3.20+ and plugin's Chakra 
3.34
   
   The plugin already reads `globalThis.ChakraUISystem` for the theme system, 
which is good. Either externalize the Chakra/Emotion packages too (relying on 
the host's copies via globals), or document that double-bundling is intentional 
for provider independence.



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.tsx:
##########
@@ -0,0 +1,350 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import {
+  Badge,
+  Box,
+  Button,
+  Flex,
+  HStack,
+  Heading,
+  Spinner,
+  Text,
+  Textarea,
+  VStack,
+} from "@chakra-ui/react";
+import {
+  type FC,
+  type KeyboardEvent,
+  useCallback,
+  useEffect,
+  useRef,
+  useState,
+} from "react";
+
+import { MessageBubble } from "src/components/MessageBubble";
+import { NoSession } from "src/components/NoSession";
+import { useSession } from "src/hooks/useSession";
+
+interface ChatPageProps {
+  dagId: string;
+  runId: string;
+  taskId: string;
+  mapIndex: number;
+}
+
+type ConfirmAction = "approve" | "reject" | null;
+
+const STATUS_BADGE: Record<
+  string,
+  { colorPalette: "green" | "red" | "yellow" | "blue"; label: string }
+> = {
+  pending_review: { colorPalette: "yellow", label: "Pending Review" },
+  approved: { colorPalette: "green", label: "Approved" },
+  rejected: { colorPalette: "red", label: "Rejected" },
+  changes_requested: { colorPalette: "blue", label: "Regenerating..." },
+  max_iterations_exceeded: { colorPalette: "red", label: "Max iterations 
exceeded" },
+  timeout_exceeded: { colorPalette: "red", label: "Timeout exceeded" },
+};
+
+export const ChatPage: FC<ChatPageProps> = ({ dagId, runId, taskId, mapIndex 
}) => {
+  const { session, loading, sendFeedback, approve, reject, refetch } =
+    useSession(dagId, runId, taskId, mapIndex);
+
+  const [feedbackText, setFeedbackText] = useState("");
+  const [confirmAction, setConfirmAction] = useState<ConfirmAction>(null);
+  const [toast, setToast] = useState<{ msg: string; ok: boolean } | 
null>(null);
+  const [isSending, setIsSending] = useState(false);
+  const chatRef = useRef<HTMLDivElement>(null);
+  const textareaRef = useRef<HTMLTextAreaElement>(null);
+  const prevConvLenRef = useRef(0);
+
+  useEffect(() => {
+    if (!session?.conversation || !chatRef.current) return;
+    const len = session.conversation.length;
+    if (len > prevConvLenRef.current) {
+      chatRef.current.scrollTop = chatRef.current.scrollHeight;
+    }
+    prevConvLenRef.current = len;
+  }, [session?.conversation]);
+
+  useEffect(() => {
+    if (toast) {
+      const t = setTimeout(() => setToast(null), 3000);
+      return () => clearTimeout(t);
+    }
+  }, [toast]);
+
+  const autoResize = useCallback(() => {
+    const ta = textareaRef.current;
+    if (ta) {
+      ta.style.height = "auto";
+      ta.style.height = `${ta.scrollHeight}px`;
+    }
+  }, []);
+
+  const handleSend = useCallback(async () => {
+    const text = feedbackText.trim();
+    if (!text || isSending) return;
+    setIsSending(true);
+    try {
+      await sendFeedback(text);
+      setFeedbackText("");
+      setToast({ msg: "Feedback sent", ok: true });
+    } catch (err) {
+      setToast({ msg: err instanceof Error ? err.message : "Error", ok: false 
});
+    } finally {
+      setIsSending(false);
+    }
+  }, [feedbackText, sendFeedback, isSending]);
+
+  const handleKeyDown = useCallback(
+    (e: KeyboardEvent) => {
+      if (e.ctrlKey && e.key === "Enter") {
+        void handleSend();
+      }
+    },
+    [handleSend],
+  );
+
+  const execConfirm = useCallback(async () => {
+    const action = confirmAction;
+    setConfirmAction(null);
+    if (!action || isSending) return;
+    setIsSending(true);
+    try {
+      if (action === "approve") {
+        await approve();
+        setToast({ msg: "Approved", ok: true });
+      } else if (action === "reject") {
+        await reject();
+        setToast({ msg: "Rejected", ok: true });
+      }
+    } catch (err) {
+      setToast({ msg: err instanceof Error ? err.message : "Error", ok: false 
});
+    } finally {
+      setIsSending(false);
+    }
+  }, [confirmAction, approve, reject, isSending]);
+
+  if (loading) {
+    return (
+      <Flex
+        align="center"
+        bg="bg"
+        color="fg"
+        flexDirection="column"
+        h="100vh"
+        justify="center"
+        p={5}
+      >
+        <Box
+          bg="bg.subtle"
+          borderRadius="xl"
+          borderWidth="1px"
+          maxW="440px"
+          p={12}
+          textAlign="center"
+        >
+          <Spinner colorPalette="brand" mb={4} size="lg" />
+          <Heading size="md">Connecting to session</Heading>
+          <Text color="fg.muted" fontSize="sm" mt={2}>
+            Looking up the HITL review session for this task...
+          </Text>
+        </Box>
+      </Flex>
+    );
+  }
+
+  if (!session) {
+    return <NoSession onRefetch={refetch} />;
+  }
+
+  const isTerminal =
+    session.status === "approved" ||
+    session.status === "rejected" ||
+    session.status === "max_iterations_exceeded" ||
+    session.status === "timeout_exceeded" ||
+    session.task_completed;
+  const canAct = session.status === "pending_review" && 
!session.task_completed;
+  const badge = STATUS_BADGE[session.status] ?? STATUS_BADGE["pending_review"];
+
+  return (
+    <Box
+      bg="bg"
+      color="fg"
+      display="flex"
+      flexDirection="column"
+      h="100vh"
+      maxW="860px"
+      mx="auto"
+    >
+      <Box borderBottomWidth="1px" px={5} py={4}>
+        <Heading size="sm">HITL Review</Heading>
+        <HStack flexWrap="wrap" fontSize="sm" gap={3} mt={1} color="fg.muted">
+          <Text as="span">
+            <Text as="b">Task:</Text> {session.task_id}
+          </Text>
+          <Text as="span">
+            <Text as="b">DAG:</Text> {session.dag_id}
+          </Text>
+          <Text as="span">
+            <Text as="b">Iteration:</Text> 
{session.iteration}/{session.max_iterations}
+          </Text>
+          <Badge colorPalette={badge.colorPalette} fontSize="2xs" px={2} 
py={0.5} borderRadius="full">
+            {badge.label}
+          </Badge>
+        </HStack>
+      </Box>
+
+      <Box flex={1} overflowY="auto" p={5} display="flex" 
flexDirection="column" gap={3} ref={chatRef}>
+        {session.conversation.map((entry) => (
+          <MessageBubble key={`${entry.role}-${entry.iteration}`} 
entry={entry} />
+        ))}
+        {session.status === "pending_review" && !isTerminal && (
+          <Text color="fg.muted" fontSize="sm" textAlign="center" py={4}>
+            Waiting for your review
+          </Text>
+        )}
+      </Box>
+
+      {isTerminal && (
+        <Box px={5} py={3}>
+          <Box
+            bg={
+              session.status === "rejected" ||
+              session.status === "max_iterations_exceeded" ||
+              session.status === "timeout_exceeded"
+                ? "red.subtle"
+                : "green.subtle"
+            }
+            color={
+              session.status === "rejected" ||
+              session.status === "max_iterations_exceeded" ||
+              session.status === "timeout_exceeded"
+                ? "red.fg"
+                : "green.fg"
+            }
+            borderRadius="xl"
+            fontWeight="semibold"
+            fontSize="md"
+            py={5}
+            textAlign="center"
+          >
+            {session.status === "approved"
+              ? "Output Approved"
+              : session.status === "rejected"
+                ? "Output Rejected"
+                : session.status === "max_iterations_exceeded"
+                  ? "Max iterations exceeded"
+                  : session.status === "timeout_exceeded"
+                    ? "Timeout exceeded"
+                    : "Task Completed — Session Ended"}
+          </Box>
+        </Box>
+      )}
+
+      {!isTerminal && (
+        <Box borderTopWidth="1px" px={5} py={3}>
+          {confirmAction === null ? (
+            <VStack align="stretch" gap={3}>
+              <HStack align="flex-end" gap={2}>
+                <Textarea
+                  ref={textareaRef}
+                  flex={1}
+                  minH="44px"
+                  maxH="160px"
+                  placeholder="Type feedback for the LLM... (Ctrl+Enter to 
send)"
+                  resize="none"
+                  rows={1}
+                  value={feedbackText}
+                  disabled={!canAct || isSending}
+                  onChange={(e) => {
+                    setFeedbackText(e.target.value);
+                    autoResize();
+                  }}
+                  onKeyDown={handleKeyDown}
+                />
+                <Button
+                  colorPalette="brand"
+                  disabled={!canAct || isSending}
+                  onClick={() => void handleSend()}
+                >
+                  Send
+                </Button>
+              </HStack>
+              <HStack gap={2}>
+                <Button
+                  colorPalette="green"
+                  disabled={!canAct || isSending}
+                  onClick={() => setConfirmAction("approve")}
+                >
+                  Approve
+                </Button>
+                <Button
+                  colorPalette="red"
+                  disabled={!canAct || isSending}
+                  onClick={() => setConfirmAction("reject")}
+                >
+                  Reject
+                </Button>
+              </HStack>
+            </VStack>
+          ) : (
+            <HStack align="center" gap={3} py={3}>
+              <Text fontSize="sm" fontWeight="medium">
+                Are you sure you want to{" "}
+                {confirmAction === "approve"
+                  ? "approve this output"
+                  : "reject this output"}
+                ?
+              </Text>
+              <HStack gap={2}>
+                <Button colorPalette="green" disabled={isSending} onClick={() 
=> void execConfirm()}>
+                  Yes
+                </Button>
+                <Button variant="outline" onClick={() => 
setConfirmAction(null)}>
+                  Cancel
+                </Button>
+              </HStack>
+            </HStack>
+          )}
+        </Box>
+      )}
+
+      {toast && (

Review Comment:
   Nit: This hand-rolled toast (`position: fixed` + `setTimeout`) works, but 
Chakra v3's `createToaster()` gives you accessible `aria-live` announcements, 
stacking when multiple toasts fire, and consistent styling — all for free. 
Minor, but worth considering.



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.tsx:
##########
@@ -0,0 +1,350 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import {
+  Badge,
+  Box,
+  Button,
+  Flex,
+  HStack,
+  Heading,
+  Spinner,
+  Text,
+  Textarea,
+  VStack,
+} from "@chakra-ui/react";
+import {
+  type FC,
+  type KeyboardEvent,
+  useCallback,
+  useEffect,
+  useRef,
+  useState,
+} from "react";
+
+import { MessageBubble } from "src/components/MessageBubble";
+import { NoSession } from "src/components/NoSession";
+import { useSession } from "src/hooks/useSession";
+
+interface ChatPageProps {
+  dagId: string;
+  runId: string;
+  taskId: string;
+  mapIndex: number;
+}
+
+type ConfirmAction = "approve" | "reject" | null;
+
+const STATUS_BADGE: Record<
+  string,
+  { colorPalette: "green" | "red" | "yellow" | "blue"; label: string }
+> = {
+  pending_review: { colorPalette: "yellow", label: "Pending Review" },
+  approved: { colorPalette: "green", label: "Approved" },
+  rejected: { colorPalette: "red", label: "Rejected" },
+  changes_requested: { colorPalette: "blue", label: "Regenerating..." },
+  max_iterations_exceeded: { colorPalette: "red", label: "Max iterations 
exceeded" },
+  timeout_exceeded: { colorPalette: "red", label: "Timeout exceeded" },
+};
+
+export const ChatPage: FC<ChatPageProps> = ({ dagId, runId, taskId, mapIndex 
}) => {
+  const { session, loading, sendFeedback, approve, reject, refetch } =

Review Comment:
   Low: `error` from `useSession` is not destructured or displayed anywhere in 
`ChatPage`. If the API returns an error (network failure, 500, etc.), the user 
sees stale data with no indication that something went wrong.
   
   At minimum, show an error banner when `error` is non-null — e.g. a colored 
`Box` at the top of the chat area.



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/NoSession.tsx:
##########
@@ -0,0 +1,84 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Box, Code, Text, VStack } from "@chakra-ui/react";
+import { type FC, useEffect } from "react";
+
+const POLL_INTERVAL_MS = 5000;
+
+interface NoSessionProps {
+  /** When provided, polls periodically to check if a session appears. */
+  onRefetch?: () => Promise<void>;
+}
+
+export const NoSession: FC<NoSessionProps> = ({ onRefetch }) => {
+  useEffect(() => {

Review Comment:
   Low: This `useEffect` polls `onRefetch` every 5 seconds, but `useSession` 
already polls `fetchSession` every 3 seconds via its own `setInterval`. When 
`useSession` gets a successful response, it'll set `session` to non-null and 
`ChatPage` will switch from `NoSession` to the chat view automatically.
   
   This polling is redundant — the `NoSession` component can just show the 
empty state without its own interval.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to