fix: address final review findings

- Finding 1: add AppState::authed_get/authed_post helpers that attach
  Bearer token when present; all 4 commands now use them
- Finding 2: MessageInput accepts onSent callback, calls it after
  successful send; ChatPage passes refreshKey bump as onSent
- Finding 3: MessageList useEffect uses cancellation flag so stale
  in-flight responses cannot overwrite a newly selected conversation
- Finding 4: scrollIntoView moved to separate effect keyed on
  messages.length so scroll only fires when new messages arrive
This commit is contained in:
2026-08-12 17:58:02 +02:00
parent 91260c8e30
commit 455d304885
7 changed files with 56 additions and 17 deletions

View File

@@ -7,8 +7,7 @@ pub async fn list_conversations(
workspace_id: String, workspace_id: String,
) -> Result<Vec<Conversation>, String> { ) -> Result<Vec<Conversation>, String> {
state state
.client .authed_get(&format!("{}/conversations?workspace_id={}", BASE_URL, workspace_id))
.get(format!("{}/conversations?workspace_id={}", BASE_URL, workspace_id))
.send() .send()
.await .await
.map_err(|e| e.to_string())? .map_err(|e| e.to_string())?

View File

@@ -7,8 +7,7 @@ pub async fn list_messages(
conversation_id: String, conversation_id: String,
) -> Result<Vec<Message>, String> { ) -> Result<Vec<Message>, String> {
state state
.client .authed_get(&format!("{}/conversations/{}/messages", BASE_URL, conversation_id))
.get(format!("{}/conversations/{}/messages", BASE_URL, conversation_id))
.send() .send()
.await .await
.map_err(|e| e.to_string())? .map_err(|e| e.to_string())?
@@ -37,8 +36,7 @@ pub async fn send_message(
}); });
state state
.client .authed_post(&format!("{}/messages", BASE_URL))
.post(format!("{}/messages", BASE_URL))
.json(&body) .json(&body)
.send() .send()
.await .await

View File

@@ -4,8 +4,7 @@ use crate::state::{AppState, BASE_URL};
#[tauri::command] #[tauri::command]
pub async fn list_workspaces(state: tauri::State<'_, AppState>) -> Result<Vec<Workspace>, String> { pub async fn list_workspaces(state: tauri::State<'_, AppState>) -> Result<Vec<Workspace>, String> {
state state
.client .authed_get(&format!("{}/workspaces", BASE_URL))
.get(format!("{}/workspaces", BASE_URL))
.send() .send()
.await .await
.map_err(|e| e.to_string())? .map_err(|e| e.to_string())?

View File

@@ -16,4 +16,21 @@ impl AppState {
user_id: Mutex::new(None), user_id: Mutex::new(None),
} }
} }
pub fn authed_get(&self, url: &str) -> reqwest::RequestBuilder {
let builder = self.client.get(url);
// ponytail: omit header when no token — server may not require auth yet
match self.token.lock().unwrap().clone() {
Some(t) => builder.bearer_auth(t),
None => builder,
}
}
pub fn authed_post(&self, url: &str) -> reqwest::RequestBuilder {
let builder = self.client.post(url);
match self.token.lock().unwrap().clone() {
Some(t) => builder.bearer_auth(t),
None => builder,
}
}
} }

View File

@@ -1,7 +1,12 @@
import { useState } from "react"; import { useState } from "react";
import { invoke } from "@tauri-apps/api/core"; import { invoke } from "@tauri-apps/api/core";
export default function MessageInput({ conversationId }: { conversationId: string }) { interface Props {
conversationId: string;
onSent: () => void;
}
export default function MessageInput({ conversationId, onSent }: Props) {
const [content, setContent] = useState(""); const [content, setContent] = useState("");
const [error, setError] = useState<string | null>(null); const [error, setError] = useState<string | null>(null);
@@ -11,6 +16,7 @@ export default function MessageInput({ conversationId }: { conversationId: strin
await invoke("send_message", { conversationId, content }); await invoke("send_message", { conversationId, content });
setContent(""); setContent("");
setError(null); setError(null);
onSent();
} catch (e) { } catch (e) {
setError(String(e)); setError(String(e));
} }

View File

@@ -8,25 +8,41 @@ interface Message {
created_at: string; created_at: string;
} }
export default function MessageList({ conversationId }: { conversationId: string }) { interface Props {
conversationId: string;
refreshKey: number;
}
export default function MessageList({ conversationId, refreshKey }: Props) {
const [messages, setMessages] = useState<Message[]>([]); const [messages, setMessages] = useState<Message[]>([]);
const [error, setError] = useState<string | null>(null); const [error, setError] = useState<string | null>(null);
const bottomRef = useRef<HTMLDivElement>(null); const bottomRef = useRef<HTMLDivElement>(null);
const prevLengthRef = useRef(0);
// Fix 3: cancellation flag prevents stale responses from overwriting new conversation
useEffect(() => { useEffect(() => {
let cancelled = false;
const load = () => const load = () =>
invoke<Message[]>("list_messages", { conversationId }) invoke<Message[]>("list_messages", { conversationId })
.then((msgs) => { .then((msgs) => {
setMessages(msgs); if (!cancelled) setMessages(msgs);
bottomRef.current?.scrollIntoView({ behavior: "smooth" });
}) })
.catch((e) => setError(String(e))); .catch((e) => { if (!cancelled) setError(String(e)); });
load(); load();
// ponytail: polling every 3s — replace with WebSocket when server supports it // ponytail: polling every 3s — replace with WebSocket when server supports it
const id = setInterval(load, 3000); const id = setInterval(load, 3000);
return () => clearInterval(id); return () => { cancelled = true; clearInterval(id); };
}, [conversationId]); // Fix 2: refreshKey in deps triggers immediate reload after send
}, [conversationId, refreshKey]);
// Fix 4: scroll only when message count increases, not on every poll
useEffect(() => {
if (messages.length > prevLengthRef.current) {
bottomRef.current?.scrollIntoView({ behavior: "smooth" });
prevLengthRef.current = messages.length;
}
}, [messages.length]);
return ( return (
<div style={{ flex: 1, overflowY: "auto", padding: 16 }}> <div style={{ flex: 1, overflowY: "auto", padding: 16 }}>

View File

@@ -7,6 +7,7 @@ import MessageInput from "../components/MessageInput";
export default function ChatPage() { export default function ChatPage() {
const { workspaceId } = useParams<{ workspaceId: string }>(); const { workspaceId } = useParams<{ workspaceId: string }>();
const [conversationId, setConversationId] = useState<string | null>(null); const [conversationId, setConversationId] = useState<string | null>(null);
const [refreshKey, setRefreshKey] = useState(0);
return ( return (
<div style={{ display: "flex", height: "100vh" }}> <div style={{ display: "flex", height: "100vh" }}>
@@ -18,8 +19,11 @@ export default function ChatPage() {
<div style={{ flex: 1, display: "flex", flexDirection: "column" }}> <div style={{ flex: 1, display: "flex", flexDirection: "column" }}>
{conversationId ? ( {conversationId ? (
<> <>
<MessageList conversationId={conversationId} /> <MessageList conversationId={conversationId} refreshKey={refreshKey} />
<MessageInput conversationId={conversationId} /> <MessageInput
conversationId={conversationId}
onSent={() => setRefreshKey((k) => k + 1)}
/>
</> </>
) : ( ) : (
<div style={{ flex: 1, display: "flex", alignItems: "center", justifyContent: "center", color: "#888" }}> <div style={{ flex: 1, display: "flex", alignItems: "center", justifyContent: "center", color: "#888" }}>