kaxil commented on code in PR #63081: URL: https://github.com/apache/airflow/pull/63081#discussion_r2901190749
########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.tsx: ########## @@ -0,0 +1,290 @@ +/*! + * 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 { + 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"; + +import styles from "./ChatPage.module.css"; + +interface ChatPageProps { + dagId: string; + runId: string; + taskId: string; + mapIndex: number; +} + +type ConfirmAction = "approve" | "reject" | null; + +const STATUS_BADGE: Record<string, { cls: string; label: string }> = { + pending_review: { cls: styles.badgePending!, label: "Pending Review" }, + approved: { cls: styles.badgeApproved!, label: "Approved" }, + rejected: { cls: styles.badgeRejected!, label: "Rejected" }, + changes_requested: { cls: styles.badgeChanges!, label: "Regenerating..." }, +}; + +export const ChatPage: FC<ChatPageProps> = ({ dagId, runId, taskId, mapIndex }) => { + const { session, error, loading, taskActive, sendFeedback, approve, reject } = + 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 chatRef = useRef<HTMLDivElement>(null); + const textareaRef = useRef<HTMLTextAreaElement>(null); + + useEffect(() => { + if (chatRef.current) { + chatRef.current.scrollTop = chatRef.current.scrollHeight; + } + }, [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) return; + try { + await sendFeedback(text); + setFeedbackText(""); + setToast({ msg: "Feedback sent", ok: true }); + } catch (err) { + setToast({ msg: err instanceof Error ? err.message : "Error", ok: false }); + } + }, [feedbackText, sendFeedback]); + + const handleKeyDown = useCallback( + (e: KeyboardEvent) => { + if (e.ctrlKey && e.key === "Enter") { + void handleSend(); + } + }, + [handleSend], + ); + + const execConfirm = useCallback(async () => { + const action = confirmAction; + setConfirmAction(null); + 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 }); + } + }, [confirmAction, approve, reject]); + + if (loading) { + return ( + <div className={styles.placeholder}> + <div className={styles.placeholderCard}> + <div className={styles.spinner} /> + <h2 className={styles.placeholderHeading}>Connecting to session</h2> + <p className={styles.placeholderDesc}> + Looking up the HITL review session for this task... + </p> + </div> + </div> + ); + } + + if (!session) { + if (taskActive === false) { + return <NoSession />; + } + + return ( + <div className={styles.placeholder}> + <div className={styles.placeholderCard}> + <div className={styles.placeholderIcon}>💬</div> + <h2 className={styles.placeholderHeading}>Waiting for session to start</h2> + <p className={styles.placeholderDesc}> + The HITL review session has not been created yet. This usually means the + task is still starting up. This page will automatically connect once the + session is available. + </p> + <div className={styles.placeholderHint}> + <span className={styles.dot} /> + <span className={styles.dot} /> + <span className={styles.dot} /> + </div> + {error && <p className={styles.placeholderMuted}>{error}</p>} + </div> + </div> + ); + } + + const isTerminal = + session.status === "approved" || + session.status === "rejected" || + session.task_completed; + const canAct = session.status === "pending_review" && !session.task_completed; + const badge = STATUS_BADGE[session.status] ?? STATUS_BADGE["pending_review"]!; + + return ( + <div className={styles.app}> + <header className={styles.header}> + <h1 className={styles.title}>HITL Review</h1> + <div className={styles.meta}> + <span><b>Task:</b> {session.task_id}</span> + <span><b>DAG:</b> {session.dag_id}</span> + <span> + <b>Iteration:</b> {session.iteration}/{session.max_iterations} + </span> + <span className={`${styles.badge} ${badge.cls}`}>{badge.label}</span> + </div> + </header> + + <div className={styles.chat} ref={chatRef}> + {session.conversation.map((entry, idx) => ( + <MessageBubble key={idx} entry={entry} /> + ))} Review Comment: Using array index as React key: ```tsx session.conversation.map((entry, idx) => ( <MessageBubble key={idx} entry={entry} /> )) ``` If the conversation grows (new messages added at the end) this works, but if messages are ever reordered or the array shifts, React will reuse the wrong DOM nodes. Since each entry has `role` + `iteration` that form a stable identity: ```tsx <MessageBubble key={`${entry.role}-${entry.iteration}`} entry={entry} /> ``` ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/hooks/useApi.ts: ########## @@ -0,0 +1,85 @@ +/*! + * 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 type { SessionResponse } from "src/types/feedback"; + +const BASE = "/hitl-review"; Review Comment: `BASE = "/hitl-review"` is an absolute path that doesn't account for Airflow's `[api] base_url` setting. If Airflow is deployed at `https://airflow.example.com/mycompany/`, the plugin mounts at `/mycompany/hitl-review` but this JS code will call `/hitl-review/sessions/find` — wrong path, 404. The edge3 provider solves this with `_get_base_url_path()` on the Python side. For the JS side, you can detect it from `window.location.pathname` (strip everything after `/hitl-review`) or inject it as a `<meta>` tag / `data-` attribute in the HTML shell: ```python # In the HTML shell <script>window.__HITL_BASE__ = "__BASE_PREFIX__";</script> ``` ```typescript // In useApi.ts const BASE = (window as any).__HITL_BASE__ ?? "/hitl-review"; ``` Same issue applies to the HTML shell's `<script src>` tag — `_PLUGIN_PREFIX` doesn't include the root path. Use `request.scope["root_path"]` from the ASGI scope to build the correct prefix. ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/hooks/useSession.ts: ########## @@ -0,0 +1,127 @@ +/*! + * 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/hooks/useApi"; +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>; +} + +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.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) => { Review Comment: After `sendFeedback` succeeds, `setSession(data)` updates state with the server response, but the response's `conversation` array may not yet include the new human message (the mixin hasn't processed it yet). The user sees "Feedback sent" toast but their message doesn't appear in the chat until the next poll cycle (up to 3 seconds). Consider an optimistic update: immediately append the user's message to the local `conversation` array, then let the next poll reconcile with the server state: ```typescript const sendFeedback = useCallback( async (text: string) => { // Optimistic: show message immediately 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 }, ], }; }); const data = await apiRef.current.submitFeedback(text); setSession(data); }, [], ); ``` ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.tsx: ########## @@ -0,0 +1,290 @@ +/*! + * 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 { + 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"; + +import styles from "./ChatPage.module.css"; + +interface ChatPageProps { + dagId: string; + runId: string; + taskId: string; + mapIndex: number; +} + +type ConfirmAction = "approve" | "reject" | null; + +const STATUS_BADGE: Record<string, { cls: string; label: string }> = { + pending_review: { cls: styles.badgePending!, label: "Pending Review" }, + approved: { cls: styles.badgeApproved!, label: "Approved" }, + rejected: { cls: styles.badgeRejected!, label: "Rejected" }, + changes_requested: { cls: styles.badgeChanges!, label: "Regenerating..." }, +}; + +export const ChatPage: FC<ChatPageProps> = ({ dagId, runId, taskId, mapIndex }) => { + const { session, error, loading, taskActive, sendFeedback, approve, reject } = + 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 chatRef = useRef<HTMLDivElement>(null); + const textareaRef = useRef<HTMLTextAreaElement>(null); + + useEffect(() => { + if (chatRef.current) { + chatRef.current.scrollTop = chatRef.current.scrollHeight; + } + }, [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) return; + try { + await sendFeedback(text); + setFeedbackText(""); + setToast({ msg: "Feedback sent", ok: true }); + } catch (err) { + setToast({ msg: err instanceof Error ? err.message : "Error", ok: false }); + } + }, [feedbackText, sendFeedback]); + + const handleKeyDown = useCallback( + (e: KeyboardEvent) => { + if (e.ctrlKey && e.key === "Enter") { + void handleSend(); + } + }, + [handleSend], + ); + + const execConfirm = useCallback(async () => { + const action = confirmAction; + setConfirmAction(null); + try { Review Comment: None of the async handlers (`handleSend`, `execConfirm`) disable the buttons while the request is in flight. A user can double-click "Approve" and fire two `POST /sessions/approve` requests — the second one will fail with 409 (status no longer `pending_review`) and flash an error toast after the success toast. Add an `isSending` state: ```tsx const [isSending, setIsSending] = useState(false); 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]); ``` Then pass `disabled={!canAct || isSending}` to the buttons. ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/hooks/useApi.ts: ########## @@ -0,0 +1,85 @@ +/*! + * 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 type { SessionResponse } from "src/types/feedback"; Review Comment: nit: `useApi.ts` follows the `useX` naming convention which implies a React hook, but this file exports a plain factory function (`createApi`) and a class (`ApiError`) — no React hooks are used. Rename to `api.ts` to match what it actually is. ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.tsx: ########## @@ -0,0 +1,290 @@ +/*! + * 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 { + 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"; + +import styles from "./ChatPage.module.css"; + +interface ChatPageProps { + dagId: string; + runId: string; + taskId: string; + mapIndex: number; +} + +type ConfirmAction = "approve" | "reject" | null; + +const STATUS_BADGE: Record<string, { cls: string; label: string }> = { + pending_review: { cls: styles.badgePending!, label: "Pending Review" }, + approved: { cls: styles.badgeApproved!, label: "Approved" }, + rejected: { cls: styles.badgeRejected!, label: "Rejected" }, + changes_requested: { cls: styles.badgeChanges!, label: "Regenerating..." }, +}; + +export const ChatPage: FC<ChatPageProps> = ({ dagId, runId, taskId, mapIndex }) => { + const { session, error, loading, taskActive, sendFeedback, approve, reject } = + 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 chatRef = useRef<HTMLDivElement>(null); + const textareaRef = useRef<HTMLTextAreaElement>(null); + + useEffect(() => { + if (chatRef.current) { + chatRef.current.scrollTop = chatRef.current.scrollHeight; + } + }, [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) return; + try { + await sendFeedback(text); + setFeedbackText(""); + setToast({ msg: "Feedback sent", ok: true }); + } catch (err) { + setToast({ msg: err instanceof Error ? err.message : "Error", ok: false }); + } + }, [feedbackText, sendFeedback]); + + const handleKeyDown = useCallback( + (e: KeyboardEvent) => { + if (e.ctrlKey && e.key === "Enter") { + void handleSend(); + } + }, + [handleSend], + ); + + const execConfirm = useCallback(async () => { + const action = confirmAction; + setConfirmAction(null); + 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 }); + } + }, [confirmAction, approve, reject]); + + if (loading) { + return ( + <div className={styles.placeholder}> + <div className={styles.placeholderCard}> + <div className={styles.spinner} /> + <h2 className={styles.placeholderHeading}>Connecting to session</h2> + <p className={styles.placeholderDesc}> + Looking up the HITL review session for this task... + </p> + </div> + </div> + ); Review Comment: nit: the textarea placeholder says "Type feedback for the LLM..." but there's no hint that Ctrl+Enter sends. Users might hit Enter expecting to send (common in chat UIs), but that just adds a newline. Either add a hint below the textarea (e.g., "Ctrl+Enter to send") or support Enter-to-send with Shift+Enter for newlines (more standard for chat UIs). ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/NoSession.tsx: ########## @@ -0,0 +1,50 @@ +/*! + * 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 { type FC, useEffect } from "react"; + +import styles from "./NoSession.module.css"; + +const AUTO_REFRESH_MS = 5000; + +export const NoSession: FC = () => { + useEffect(() => { + const timer = setTimeout(() => location.reload(), AUTO_REFRESH_MS); Review Comment: `location.reload()` does a hard page refresh every 5 seconds. This loses all JS state (including any error context) and triggers a full HTML+JS+CSS download cycle each time. The `useSession` hook already polls the API every 3 seconds and handles the "no session yet" → "session found" transition. This component should use that same hook instead of reloading the page. Or at minimum, use `fetch` to check the API and only reload when a session actually appears: ```typescript const timer = setInterval(async () => { try { const res = await fetch(`/hitl-review/sessions/find?${qs}`); if (res.ok) location.reload(); } catch { /* ignore */ } }, AUTO_REFRESH_MS); ``` But the better fix is to not render `NoSession` as a terminal state — keep it within the `useSession` lifecycle so the transition happens via React state, not page reload. ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.module.css: ########## @@ -0,0 +1,372 @@ +/*! + * 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. + */ + +/* Licensed to the Apache Software Foundation (ASF) under one Review Comment: The Apache license header appears twice — once as a `/*! */` block (lines 1-18) and again as a `/* */` block (lines 20-36). Same duplication in `MessageBubble.module.css` and `NoSession.module.css`. Remove the second copy in all three files. ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/components/ChatPage.module.css: ########## @@ -0,0 +1,372 @@ +/*! + * 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. + */ + +/* 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. */ + +:root { + --bg: #f8f9fb; + --surface: #fff; + --border: #e2e6ea; + --text: #1a1d21; + --text-muted: #6b7280; + --accent: #3b82f6; + --accent-hover: #2563eb; + --approve: #16a34a; + --approve-hover: #15803d; + --reject: #dc2626; + --reject-hover: #b91c1c; + --bubble-ai: #f0f4f8; + --bubble-human: #3b82f6; + --bubble-human-text: #fff; + --radius: 12px; + --shadow: 0 1px 3px rgba(0, 0, 0, 0.08); + --font: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, + Arial, sans-serif; +} + Review Comment: Dark mode is driven by `@media (prefers-color-scheme: dark)` which follows the OS preference. But the Airflow UI has its own dark mode toggle (independent of OS). When a user has OS light mode + Airflow dark mode (or vice versa), the HITL chat page will use the wrong theme. For an initial implementation this is acceptable, but worth noting as a follow-up. One approach: read a `data-theme` attribute or CSS class from the parent (if iframe) or inject the current Airflow theme preference into the HTML shell. ########## providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/main.tsx: ########## @@ -0,0 +1,59 @@ +/*! + * 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 { StrictMode, type FC } from "react"; +import { createRoot } from "react-dom/client"; + +import { ChatPage } from "src/components/ChatPage"; +import { NoSession } from "src/components/NoSession"; + +export type PluginComponentProps = object; + +/** + * Main plugin component. + * + * Reads dag_id / run_id / task_id from the URL search params injected by + * the Airflow external_views iframe. If parameters are missing it shows + * the "no session" fallback. + */ +const PluginComponent: FC<PluginComponentProps> = () => { + const params = new URLSearchParams(globalThis.location.search); + const dagId = params.get("dag_id") ?? ""; + const runId = (params.get("run_id") ?? "").replace(/ /g, "+"); Review Comment: nit: `.replace(/ /g, "+")` on `run_id` here mirrors the same issue as the Python side (`run_id.replace(" ", "+")` in the `/chat` endpoint). Both suggest a URL encoding issue where `+` in query params is decoded as space. If this normalization is needed, it should happen in one canonical place. Since `useApi.ts:buildQs` uses `encodeURIComponent(runId)` which encodes spaces as `%20` (not `+`), this `replace` is working around a different code path that sends `+` as space. Track down where `+` becomes space and fix it there instead. -- 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]
