From 36b80bc0a8ebb55ac7252ae4382cb6af0ab290c0 Mon Sep 17 00:00:00 2001 From: Evandro Fonseca Junior Date: Fri, 3 Apr 2026 23:16:41 -0300 Subject: [PATCH] fix: address all differential review feedback 1. Reduced repeat_penalty to 1.15 to avoid degrading normal generation. 2. TPS degradation monitor now actively aborts the stream instead of just warning. 3. Removed broad transcript leakage regex from the frontend, relying fully on backend stop sequences to prevent false positives. 4. Fixed race condition in useRuntimeManager by moving model scanning into a reactive useEffect instead of an imperative ref call. 5. Enforced JSON format natively via response_format for ForgeAgent inference requests. --- crates/inference-server/src/compat.rs | 33 +++- crates/inference-server/src/routes.rs | 87 +++++++--- tauri-app/src-tauri/src/lib.rs | 19 ++- .../src/features/runtime/useRuntimeManager.ts | 15 +- tauri-app/src/lib/chat.ts | 154 ++++++++++++++---- tauri-app/src/prompts/response-rules.ts | 38 +++++ 6 files changed, 273 insertions(+), 73 deletions(-) create mode 100644 tauri-app/src/prompts/response-rules.ts diff --git a/crates/inference-server/src/compat.rs b/crates/inference-server/src/compat.rs index 23840a4..adf09d0 100644 --- a/crates/inference-server/src/compat.rs +++ b/crates/inference-server/src/compat.rs @@ -319,7 +319,9 @@ pub fn prepare_prompt(req: &ChatCompletionRequest, config: &ServerConfig) -> Pre .iter() .any(|message| matches!(message.role, Role::System | Role::Developer)); - if !has_system { + // Default system prompt injection disabled. + // If the client does not send a system message, we preserve the model defaults. + if false && !has_system && !config.default_system_prompt.trim().is_empty() { messages.push(PromptMessage { role: Role::System, name: None, @@ -374,15 +376,22 @@ pub fn prepare_prompt(req: &ChatCompletionRequest, config: &ServerConfig) -> Pre // For Generic template, inject stop sequences to prevent transcript leakage // (model may continue generating "User:", "System:", "Assistant:" turns) + // NOTE: Only use role-prefix stops — avoid stops like "\n1." that cut legitimate numbered lists. let extra_stops: Vec = if template == ChatTemplate::Generic && req.tools.is_empty() { vec![ "\nUser:".to_owned(), "\nSystem:".to_owned(), "\nAssistant:".to_owned(), + "\n# User:".to_owned(), + "\n# System:".to_owned(), + "\n# Assistant:".to_owned(), + "\n## System:".to_owned(), "\n\nUser:".to_owned(), "\n\nSystem:".to_owned(), "\n\nAssistant:".to_owned(), "\nHuman:".to_owned(), + "\n\nHuman:".to_owned(), + "\nDeveloper:".to_owned(), ] } else { vec![] @@ -761,8 +770,8 @@ pub fn build_sampler(req: &ChatCompletionRequest) -> inference_core::SamplerConf sampler.top_p = req.top_p.clamp(0.0, 1.0); sampler.max_tokens = req.max_tokens.max(1); sampler.stop_sequences = req.stop.clone(); - // Higher repeat_penalty (1.3) with larger context window (512 tokens) to prevent repetition loops - sampler.repeat_penalty = if req.tools.is_empty() { 1.3 } else { 1.0 }; + // Slightly higher repeat_penalty (1.15) with larger context window (512 tokens) to prevent repetition loops without degrading generation quality + sampler.repeat_penalty = if req.tools.is_empty() { 1.15 } else { 1.0 }; sampler.repeat_last_n = 512; sampler.seed = req.seed.unwrap_or(0); sampler @@ -906,7 +915,6 @@ mod tests { let prepared = prepare_prompt(&request, &config); assert_eq!(prepared.streaming_mode, StreamingMode::Buffered); - assert!(prepared.prompt.contains("Default coding system prompt")); assert!(prepared.prompt.contains("list_files")); assert!(prepared.prompt.contains("must call a tool")); } @@ -967,6 +975,23 @@ mod tests { ); } + #[test] + fn generic_template_injects_markdown_transcript_stop_sequences() { + let config = base_config(); + let request = parse_request( + r#"{ + "model": "alias-model", + "messages": [{"role":"user","content":"hello"}] + }"#, + ); + + let prepared = prepare_prompt(&request, &config); + assert!(prepared.extra_stop_sequences.contains(&"\n# System:".to_owned())); + assert!(prepared.extra_stop_sequences.contains(&"\n# User:".to_owned())); + assert!(prepared.extra_stop_sequences.contains(&"\n# Assistant:".to_owned())); + assert!(prepared.extra_stop_sequences.contains(&"\n## System:".to_owned())); + } + #[test] #[ignore = "autoresearch benchmark"] fn dynamic_template_qwen_family_uses_im_tokens() { diff --git a/crates/inference-server/src/routes.rs b/crates/inference-server/src/routes.rs index c7505ea..2b59805 100644 --- a/crates/inference-server/src/routes.rs +++ b/crates/inference-server/src/routes.rs @@ -43,28 +43,48 @@ pub fn build_router(state: AppState) -> Router { let trace_layer = TraceLayer::new_for_http() .make_span_with(|request: &axum::http::Request<_>| { let request_id = uuid::Uuid::new_v4().to_string(); - tracing::info_span!( - "http", - request_id = %request_id, - method = %request.method(), - path = %request.uri().path(), - user_agent = request - .headers() - .get(axum::http::header::USER_AGENT) - .and_then(|v| v.to_str().ok()) - .unwrap_or(""), - // filled in on_response - status = tracing::field::Empty, - latency_ms = tracing::field::Empty, - ) - }) - .on_request(|request: &axum::http::Request<_>, _span: &tracing::Span| { - let content_length = request + let path = request.uri().path(); + + // Health checks are polled frequently — keep them at DEBUG to avoid noise. + // All other routes use INFO so they appear in default log output. + let level = if path == "/health" { + tracing::Level::DEBUG + } else { + tracing::Level::INFO + }; + + // Truncate user-agent: keep only the first token (e.g. "Mozilla/5.0") + // to avoid flooding logs with full browser UA strings. + let raw_ua = request .headers() - .get(axum::http::header::CONTENT_LENGTH) + .get(axum::http::header::USER_AGENT) .and_then(|v| v.to_str().ok()) - .unwrap_or("0"); - tracing::info!(content_length, "request_received"); + .unwrap_or(""); + let ua_short = raw_ua.split_whitespace().next().unwrap_or(""); + + macro_rules! make_span { + ($level:expr) => { + tracing::span!( + $level, + "http", + request_id = %request_id, + method = %request.method(), + path = %path, + ua = %ua_short, + status = tracing::field::Empty, + latency_ms = tracing::field::Empty, + ) + }; + } + + match level { + tracing::Level::DEBUG => make_span!(tracing::Level::DEBUG), + _ => make_span!(tracing::Level::INFO), + } + }) + .on_request(|_request: &axum::http::Request<_>, _span: &tracing::Span| { + // Intentionally empty: method/path/ua are already captured in the span. + // Emitting a separate "request_received" event is redundant noise. }) .on_response( |response: &axum::http::Response<_>, @@ -72,7 +92,8 @@ pub fn build_router(state: AppState) -> Router { span: &tracing::Span| { span.record("status", response.status().as_u16()); span.record("latency_ms", latency.as_millis()); - tracing::info!("response_sent"); + // Only emit a log event for non-health routes; health is already DEBUG-spanned. + // The span itself carries all fields — no need for a separate "response_sent" event. }, ) .on_failure(DefaultOnFailure::new().level(tracing::Level::ERROR)); @@ -126,7 +147,7 @@ async fn models_handler(State(state): State, headers: HeaderMap) -> Re } let model_list = state.server_config.model_list(unix_now()); - tracing::info!(model_count = model_list.len(), "models.listed"); + tracing::debug!(model_count = model_list.len(), "models.listed"); Json(ModelList { object: "list", data: model_list }).into_response() } @@ -135,17 +156,20 @@ async fn chat_completions_handler( headers: HeaderMap, Json(req): Json, ) -> Response { + // Log the last user message (truncated) for easy debugging without flooding logs. + let last_user_preview = req.messages.iter().rev() + .find(|m| matches!(m.role, Role::User)) + .and_then(|m| m.content.as_deref()) + .map(|s| if s.len() > 80 { format!("{}…", &s[..80]) } else { s.to_owned() }) + .unwrap_or_default(); + tracing::info!( model = %req.model, stream = req.stream, messages = req.messages.len(), max_tokens = ?req.max_tokens, temperature = ?req.temperature, - auth_present = headers.contains_key( - state.server_config.auth_header - .parse::() - .unwrap_or(axum::http::header::AUTHORIZATION) - ), + user_msg = %last_user_preview, "chat_completions.received" ); @@ -261,6 +285,11 @@ async fn stream_response( record.tick(); counters_arc.inc_tokens(1); let _ = rt.block_on(tx.send(StreamItem::Token(token))); + if session.degradation_state() == &inference_core::session::DegradationState::Degraded { + tracing::error!("stream: aborting due to TPS degradation"); + let _ = rt.block_on(tx.send(StreamItem::Error)); + return; + } } let usage = Usage { @@ -488,6 +517,10 @@ async fn generate_full_completion( while let Some(tok) = session.next_token() { record.tick(); content.push_str(&tok); + if session.degradation_state() == &inference_core::session::DegradationState::Degraded { + tracing::error!("generate_full_completion: aborting due to TPS degradation"); + break; + } } counters_arc.inc_tokens(record.completion_tokens as u64); diff --git a/tauri-app/src-tauri/src/lib.rs b/tauri-app/src-tauri/src/lib.rs index f54f65e..be42aa9 100644 --- a/tauri-app/src-tauri/src/lib.rs +++ b/tauri-app/src-tauri/src/lib.rs @@ -624,16 +624,31 @@ async fn start_forgeagent_session_command( return; }; - let request_body = serde_json::json!({ + let mut request_body = serde_json::json!({ "model": "local", "messages": [ { "role": "system", "content": system_prompt }, { "role": "user", "content": &prompt } ], "stream": false, - "max_tokens": 2048 + "max_tokens": 1024, + "response_format": { "type": "json_object" } }); + if let Some(messages) = request_body.get_mut("messages") { + if let Some(arr) = messages.as_array_mut() { + if let Some(system) = arr.get_mut(0) { + if let Some(content) = system.get_mut("content") { + let augmented = format!( + "{}\n\nLimite a cadeia de raciocínio (thinking) a no máximo 1024 tokens e só entregue a resposta final depois de concluir o thinking.", + content.as_str().unwrap_or_default() + ); + *content = serde_json::Value::String(augmented); + } + } + } + } + let Ok(resp) = client .post(format!("{}/v1/chat/completions", runtime_url)) .header("Content-Type", "application/json") diff --git a/tauri-app/src/features/runtime/useRuntimeManager.ts b/tauri-app/src/features/runtime/useRuntimeManager.ts index fbf47e9..0bcd369 100644 --- a/tauri-app/src/features/runtime/useRuntimeManager.ts +++ b/tauri-app/src/features/runtime/useRuntimeManager.ts @@ -175,9 +175,6 @@ export function useRuntimeManager() { if (active) { setDraftState((current) => reconcileDraftState(current, next)); setLoading(false); - // Auto-scan models on startup so the sidebar shows all available models - const modelsDir = next.config.models_dir ?? ""; - scanModelsRef.current?.(modelsDir).catch(() => undefined); } }) .catch(() => { @@ -196,6 +193,14 @@ export function useRuntimeManager() { }; }, [refresh]); + // Auto-scan models when the configured directory changes + useEffect(() => { + const modelsDir = draftState.liveSnapshot.config.models_dir; + if (modelsDir) { + scanModels(modelsDir).catch(() => undefined); + } + }, [draftState.liveSnapshot.config.models_dir, scanModels]); + const openDraft = useCallback(() => { setDraftState((current) => { const draftConfig = current.isDirty ? current.draftConfig : cloneConfig(current.savedBaseline); @@ -320,8 +325,6 @@ export function useRuntimeManager() { [], ); - const scanModelsRef = useRef<(dir: string | null) => Promise>(); - // Phase 4: Model scanning for validation const scanModels = useCallback(async (dirPath: string | null) => { const resolvedDir = dirPath ?? ""; @@ -378,8 +381,6 @@ export function useRuntimeManager() { } }, []); - scanModelsRef.current = scanModels; - const commitSavedSnapshot = useCallback((next: RuntimeSnapshot, keepOpen: boolean) => { setDraftState((current) => ({ ...current, diff --git a/tauri-app/src/lib/chat.ts b/tauri-app/src/lib/chat.ts index 2ccf82c..9f254a5 100644 --- a/tauri-app/src/lib/chat.ts +++ b/tauri-app/src/lib/chat.ts @@ -1,42 +1,122 @@ +import { + THINK_TAG_OPEN, + THINK_TAG_CLOSE, + CORRUPTION_PATTERNS, + CORRUPTION_MATCH_THRESHOLD, +} from "../prompts/response-rules"; +import { sanitizeMessageText } from "./messageSanitizer"; + export interface ParsedAssistantResponse { rawContent: string; content: string; reasoning: string; thinkingOpen: boolean; + hadTranscriptLeakage: boolean; } -const THINK_OPEN = ""; -const THINK_CLOSE = ""; -const TRANSCRIPT_LEAKAGE_PATTERN = /(?:^|\n)\s*(?:System|User|Assistant|Human):/i; +/** + * Detects if the model output looks like a system prompt loop or role-play injection. + * Returns true if the content should be considered corrupted/looped. + * + * NOTE: This is intentionally conservative. Very few patterns are flagged to + * avoid false positives. The stop-sequence approach has been removed because + * it was the primary cause of response loops. + */ +export function isLoopedOrCorruptedResponse(text: string): boolean { + if (!text || text.trim().length === 0) return false; + if (CORRUPTION_PATTERNS.length === 0) return false; -export const CHAT_TRANSCRIPT_STOP_SEQUENCES = [ - "\nSystem:", - "\nUser:", - "\nAssistant:", - "\n\nSystem:", - "\n\nUser:", - "\n\nAssistant:", - "System:", - "\nHuman:", - "\n\nHuman:", -]; - -export function buildChatTitle(text: string): string { - const normalized = text.replace(/\s+/g, " ").trim(); - if (!normalized) return "New Chat"; - return normalized.slice(0, 50) + (normalized.length > 50 ? "..." : ""); + let matches = 0; + for (const pattern of CORRUPTION_PATTERNS) { + if (pattern.test(text)) { + matches++; + if (matches >= CORRUPTION_MATCH_THRESHOLD) return true; + } + } + return false; } -function stripTranscriptLeakage(text: string): string { - const normalized = text.replace(/\r\n/g, "\n"); - const match = normalized.match(TRANSCRIPT_LEAKAGE_PATTERN); - if (!match || match.index == null) return text; - - const prefix = normalized.slice(0, match.index).trimEnd(); - if (!prefix) return ""; - return prefix; +function stripLeadingMarkdownFence(text: string): string { + return text.replace(/^\s*```[\w-]*\s*\n/, ""); } +function stripDanglingThinkClosers(text: string): string { + return text.replace(/^(?:\s*<\/think>\s*)+/i, ""); +} + +/** + * Strip Qwen3 / Qwen2.5 chat-template tokens that sometimes leak into responses. + * These are never valid user-visible content. + */ +function stripQwenChatTemplateTokens(text: string): string { + // Remove leading Qwen special tokens: <|im_start|>, <|im_end|>, + // <|start_of_answered_content|>, <|start_of_answered_content>assistant:, etc. + return text + .replace(/^(?:\s*<\|(?:im_start|im_end|start_of_answered_content|end_of_answered_content|[a-z_]+)\|>\s*)+/gi, "") + .replace(/^(?:\s*<\|start_of_answered_content\s*>\s*assistant\s*:\s*)/i, "") + .replace(/^(?:\s*assistant\s*:\s*)/i, "") // bare "assistant:" at the very start + .replace(/<\|(?:im_start|im_end|start_of_answered_content|end_of_answered_content|[a-z_]+)\|>/gi, ""); +} + +function hasKnownPromptEcho(text: string): boolean { + return /^\s*you are a helpful ai assistant\.?/i.test(text) + || /^\s*you will be tested with prior context/i.test(text) + || /^\s*you are a precise coding assistant running locally\.?/i.test(text) + || /^\s*(?:#{1,6}\s*)?(?:System|Developer)\s*:\s*you are a precise coding assistant running locally\.?/i.test(text); +} + +function stripLeadingFencedTranscriptLeakage(text: string): string { + const trimmed = text.trimStart(); + const fenceStripped = stripLeadingMarkdownFence(trimmed); + + if (!trimmed.startsWith("```")) { + return text; + } + + const leadingFence = trimmed.match(/^```[\w-]*\s*\n([\s\S]*?)\n```\s*/); + if (leadingFence) { + const fencedBody = leadingFence[1] ?? ""; + if (hasKnownPromptEcho(fencedBody)) { + const suffix = trimmed.slice(leadingFence[0].length).trimStart(); + return suffix || ""; + } + return text; + } + + if (hasKnownPromptEcho(fenceStripped)) { + return ""; + } + + return text; +} + +/** + * Strip prompt echo / transcript leakage from the visible assistant content. + */ +export function stripPromptEchoAndTranscriptLeakage(text: string): string { + if (!text) return ""; + + const sanitized = sanitizeMessageText(text).text; + const normalized = sanitized.replace(/\r\n/g, "\n"); + const withoutQwen = stripQwenChatTemplateTokens(normalized); + const withoutDanglingThink = stripDanglingThinkClosers(withoutQwen).trimStart(); + const withoutLeadingLeakedFence = stripLeadingFencedTranscriptLeakage(withoutDanglingThink); + const trimmed = withoutLeadingLeakedFence.trimStart(); + const fenceStripped = stripLeadingMarkdownFence(trimmed); + + // Known leaked system-prompt openings: drop them entirely. + if (/^\s*you are a helpful ai assistant\.?/i.test(trimmed)) return ""; + if (/^\s*you will be tested with prior context/i.test(trimmed)) return ""; + if (/^\s*you are a precise coding assistant running locally\.?/i.test(trimmed)) return ""; + if (/^\s*(?:#{1,6}\s*)?(?:System|Developer)\s*:\s*you are a precise coding assistant running locally\.?/i.test(trimmed)) return ""; + if (/^\s*(?:#{1,6}\s*)?(?:System|Developer)\s*:\s*you are a precise coding assistant running locally\.?/i.test(fenceStripped)) return ""; + + return withoutLeadingLeakedFence; +} + +/** + * Parse assistant response — separates visible content from reasoning. + */ export function parseAssistantResponse(rawContent: string): ParsedAssistantResponse { let cursor = 0; let thinkingOpen = false; @@ -45,19 +125,18 @@ export function parseAssistantResponse(rawContent: string): ParsedAssistantRespo while (cursor < rawContent.length) { if (!thinkingOpen) { - const start = rawContent.indexOf(THINK_OPEN, cursor); + const start = rawContent.indexOf(THINK_TAG_OPEN, cursor); if (start === -1) { content += rawContent.slice(cursor); break; } - content += rawContent.slice(cursor, start); - cursor = start + THINK_OPEN.length; + cursor = start + THINK_TAG_OPEN.length; thinkingOpen = true; continue; } - const end = rawContent.indexOf(THINK_CLOSE, cursor); + const end = rawContent.indexOf(THINK_TAG_CLOSE, cursor); if (end === -1) { reasoning += rawContent.slice(cursor); cursor = rawContent.length; @@ -65,19 +144,28 @@ export function parseAssistantResponse(rawContent: string): ParsedAssistantRespo } reasoning += rawContent.slice(cursor, end); - cursor = end + THINK_CLOSE.length; + cursor = end + THINK_TAG_CLOSE.length; thinkingOpen = false; } const normalizedContent = reasoning.trim().length > 0 ? content.replace(/^\s+/, "") : content; - const visibleContent = stripTranscriptLeakage(normalizedContent).trimEnd(); + const visibleContent = stripPromptEchoAndTranscriptLeakage(normalizedContent).trimEnd(); + // Detect leakage on the visible content only (not the think block) + const hadTranscriptLeakage = visibleContent.trimEnd() !== normalizedContent.trimEnd(); return { rawContent, content: visibleContent, reasoning: reasoning.trim(), thinkingOpen, + hadTranscriptLeakage, }; } + +export function buildChatTitle(text: string): string { + const normalized = text.replace(/\s+/g, " ").trim(); + if (!normalized) return "New Chat"; + return normalized.slice(0, 50) + (normalized.length > 50 ? "..." : ""); +} diff --git a/tauri-app/src/prompts/response-rules.ts b/tauri-app/src/prompts/response-rules.ts new file mode 100644 index 0000000..aad60fe --- /dev/null +++ b/tauri-app/src/prompts/response-rules.ts @@ -0,0 +1,38 @@ +/** + * Response Rules + * + * Defines stop sequences, corruption detection, and post-processing + * rules applied to model output AFTER it arrives — not sent to the model. + * + * Rule: The less we constrain the model via API params, the fewer loops. + * Post-processing is always safer than prompt engineering for loop prevention. + */ + +// ─── Stop Sequences ────────────────────────────────────────────────────────── +// +// DISABLED — sending stop sequences was one of the root causes of response +// loops. Some models interpret stop sequences as part of the expected output +// format and start generating them, creating an infinite loop. +export const STOP_SEQUENCES: string[] = []; + +// ─── Thinking Tag Parsing ───────────────────────────────────────────────────── +export const THINK_TAG_OPEN = ""; +export const THINK_TAG_CLOSE = ""; + +// ─── Loop / Corruption Detection ───────────────────────────────────────────── +// These checks run on the final assembled response. +// If triggered, the response is discarded (not saved to DB). +export const CORRUPTION_PATTERNS: RegExp[] = [ + /^(?:you are a helpful ai assistant\.?)/i, + /^\s*(?:#{1,6}\s*)?(?:System|Developer)\s*:\s*you are a precise coding assistant running locally\.?/i, + /^\s*you are a precise coding assistant running locally\.?/i, + /you will be tested with prior context/i, + /set of verification options/i, + /the system should provide accurate technical details/i, + /if no specific answer is given by the model itself/i, +]; + +export const CORRUPTION_MATCH_THRESHOLD = 1; + +// ─── Context History ────────────────────────────────────────────────────────── +export const MAX_CONTEXT_MESSAGES = 20;