electron-rare/Aperant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(github): enable text fallback when structured output validation fails

Browse Source 
When Claude cannot produce valid structured output matching the Pydantic
schema (error_max_structured_output_retries), the reviewers now proceed
to use text fallback parsing instead of raising an exception or
returning empty results.

Previously, when structured output validation failed:
- parallel_followup_reviewer.py raised RuntimeError, losing all findings
- parallel_orchestrator_reviewer.py returned empty findings list

Now, when structured output validation fails:
- Warning is logged indicating text fallback will be used
- structured_output is cleared to ensure fallback parsing runs
- The markdown fallback parser extracts findings from AI's text response

Also reduces verbose debug logging in frontend modules:
- Removed frequent TRACE/CACHE/buffer update logs
- Kept error logs and state-change notifications
- Debug output is now cleaner and more actionable

Files modified:
- parallel_followup_reviewer.py: Handle error_max_structured_output_retries
- parallel_orchestrator_reviewer.py: Same fix in 2 locations
- usage-monitor.ts: Removed verbose trace logs
- credential-utils.ts: Removed cache hit logs
- terminal-session-store.ts: Removed buffer update logs
- token-refresh.ts: Removed frequent validity check logs
- claude-profile-manager.ts: Removed getActiveProfile log

Co-Authored-By: Claude Opus 4.5 <[email protected]>
This commit is contained in:
Test User
2026-02-04 19:46:01 +01:00
co-authored by Claude Opus 4.5
parent 2f4393b7f1
commit 23f712c094
7 changed files with 69 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/runners/github/services/parallel_followup_reviewer.py
+21 -7
View File
@@ -567,13 +567,27 @@ The SDK will run invoked agents in parallel automatically.
)
# Check for stream processing errors
if stream_result.get("error"):
logger.error(
f"[ParallelFollowup] SDK stream failed: {stream_result['error']}"
)
raise RuntimeError(
f"SDK stream processing failed: {stream_result['error']}"
)
stream_error = stream_result.get("error")
if stream_error:
# Don't raise for structured_output_validation_failed - use text fallback instead
if stream_error == "structured_output_validation_failed":
logger.warning(
"[ParallelFollowup] Structured output validation failed after retries. "
"Will use text fallback to extract findings."
)
safe_print(
"[ParallelFollowup] Structured output failed - using text fallback",
flush=True,
)
# Clear structured_output to ensure fallback is used
stream_result["structured_output"] = None
else:
logger.error(
f"[ParallelFollowup] SDK stream failed: {stream_error}"
)
raise RuntimeError(
f"SDK stream processing failed: {stream_error}"
)
result_text = stream_result["result_text"]
structured_output = stream_result["structured_output"]
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py
+46 -24
View File
@@ -526,14 +526,27 @@ Report findings with specific file paths, line numbers, and code evidence.
error = stream_result.get("error")
if error:
logger.error(
f"[Specialist:{config.name}] SDK stream failed: {error}"
)
safe_print(
f"[Specialist:{config.name}] Analysis failed: {error}",
flush=True,
)
return (config.name, [])
# Don't bail out for structured_output_validation_failed - use text fallback
if error == "structured_output_validation_failed":
logger.warning(
f"[Specialist:{config.name}] Structured output validation failed. "
"Using text fallback."
)
safe_print(
f"[Specialist:{config.name}] Structured output failed - using text fallback",
flush=True,
)
# Clear structured_output to ensure fallback is used
stream_result["structured_output"] = None
else:
logger.error(
f"[Specialist:{config.name}] SDK stream failed: {error}"
)
safe_print(
f"[Specialist:{config.name}] Analysis failed: {error}",
flush=True,
)
return (config.name, [])
# Parse structured output
structured_output = stream_result.get("structured_output")
@@ -1780,25 +1793,34 @@ For EACH finding above:
error = stream_result.get("error")
if error:
# Check for specific error types that warrant retry
error_str = str(error).lower()
is_retryable = (
"400" in error_str
or "concurrency" in error_str
or "circuit breaker" in error_str
or "tool_use" in error_str
)
if is_retryable and attempt < MAX_VALIDATION_RETRIES:
# Handle structured output validation failure - use text fallback
if error == "structured_output_validation_failed":
logger.warning(
f"[PRReview] Retryable validation error: {error}"
"[PRReview:FindingValidator] Structured output validation failed. "
"Will use text fallback to parse validation results."
)
# Clear structured_output to trigger text parsing below
stream_result["structured_output"] = None
else:
# Check for specific error types that warrant retry
error_str = str(error).lower()
is_retryable = (
"400" in error_str
or "concurrency" in error_str
or "circuit breaker" in error_str
or "tool_use" in error_str
)
last_error = Exception(error)
continue # Retry
logger.error(f"[PRReview] Validation failed: {error}")
# Fail-safe: return original findings
return findings
if is_retryable and attempt < MAX_VALIDATION_RETRIES:
logger.warning(
f"[PRReview] Retryable validation error: {error}"
)
last_error = Exception(error)
continue # Retry
logger.error(f"[PRReview] Validation failed: {error}")
# Fail-safe: return original findings
return findings
structured_output = stream_result.get("structured_output")
apps/frontend/src/main/claude-profile-manager.ts
-7
View File
@@ -329,13 +329,6 @@ export class ClaudeProfileManager {
// Fallback to default
const defaultProfile = this.data.profiles.find(p => p.isDefault);
if (defaultProfile) {
if (process.env.DEBUG === 'true') {
console.warn('[ClaudeProfileManager] getActiveProfile - using default:', {
id: defaultProfile.id,
name: defaultProfile.name,
email: defaultProfile.email
});
}
return defaultProfile;
}
// If somehow no default exists, return first profile
apps/frontend/src/main/claude-profile/credential-utils.ts
-9
View File
@@ -616,15 +616,6 @@ function getCredentialsFromMacOSKeychain(configDir?: string, forceRefresh = fals
if (!forceRefresh && cached) {
const ttl = cached.credentials.error ? ERROR_CACHE_TTL_MS : CACHE_TTL_MS;
if ((now - cached.timestamp) < ttl) {
if (isDebug) {
const cacheAge = now - cached.timestamp;
console.warn('[CredentialUtils:macOS:CACHE] Returning cached credentials:', {
serviceName,
hasToken: !!cached.credentials.token,
tokenFingerprint: getTokenFingerprint(cached.credentials.token),
cacheAge: Math.round(cacheAge / 1000) + 's'
});
}
return cached.credentials;
}
}
apps/frontend/src/main/claude-profile/token-refresh.ts
-6
View File
@@ -319,12 +319,6 @@ export async function ensureValidToken(
? configDir.replace(/^~/, homedir())
: configDir;
if (isDebug) {
console.warn('[TokenRefresh:ensureValidToken] Checking token validity', {
configDir: expandedConfigDir || 'default'
});
}
// Step 1: Read full credentials from keychain
const creds = getFullCredentialsFromKeychain(expandedConfigDir);
apps/frontend/src/main/claude-profile/usage-monitor.ts
+2 -58
View File
@@ -90,60 +90,22 @@ const PROVIDER_USAGE_ENDPOINTS: readonly ProviderUsageEndpoint[] = [
export function getUsageEndpoint(provider: ApiProvider, baseUrl: string): string | null {
const isDebug = process.env.DEBUG === 'true';
if (isDebug) {
console.warn('[UsageMonitor:ENDPOINT_CONSTRUCTION] Constructing usage endpoint:', {
provider,
baseUrl
});
}
const endpointConfig = PROVIDER_USAGE_ENDPOINTS.find(e => e.provider === provider);
if (!endpointConfig) {
if (isDebug) {
console.warn('[UsageMonitor:ENDPOINT_CONSTRUCTION] Unknown provider - no endpoint configured:', {
provider,
availableProviders: PROVIDER_USAGE_ENDPOINTS.map(e => e.provider)
});
}
return null;
}
if (isDebug) {
console.warn('[UsageMonitor:ENDPOINT_CONSTRUCTION] Found endpoint config for provider:', {
provider,
usagePath: endpointConfig.usagePath
});
}
try {
const url = new URL(baseUrl);
const originalPath = url.pathname;
// Replace the path with the usage endpoint path
url.pathname = endpointConfig.usagePath;
// Note: quota/limit endpoint doesn't require query parameters
// The model-usage and tool-usage endpoints would need time windows, but we're using quota/limit
const finalUrl = url.toString();
if (isDebug) {
console.warn('[UsageMonitor:ENDPOINT_CONSTRUCTION] Successfully constructed endpoint:', {
provider,
originalPath,
newPath: endpointConfig.usagePath,
finalUrl
});
}
return finalUrl;
return url.toString();
} catch (error) {
console.error('[UsageMonitor] Invalid baseUrl for usage endpoint:', baseUrl);
if (isDebug) {
console.warn('[UsageMonitor:ENDPOINT_CONSTRUCTION] URL construction failed:', {
baseUrl,
error: error instanceof Error ? error.message : String(error)
});
}
return null;
}
}
@@ -163,18 +125,7 @@ export function getUsageEndpoint(provider: ApiProvider, baseUrl: string): string
*/
export function detectProvider(baseUrl: string): ApiProvider {
// Wrapper around shared detectProvider with debug logging for main process
const isDebug = process.env.DEBUG === 'true';
const provider = sharedDetectProvider(baseUrl);
if (isDebug) {
console.warn('[UsageMonitor:PROVIDER_DETECTION] Detected provider:', {
baseUrl,
provider
});
}
return provider;
return sharedDetectProvider(baseUrl);
}
/**
@@ -844,18 +795,12 @@ export class UsageMonitor extends EventEmitter {
// Fallback: Try direct keychain read (e.g., if refresh token unavailable)
const keychainCreds = getCredentialsFromKeychain(activeProfile.configDir);
if (keychainCreds.token) {
this.debugLog('[UsageMonitor:TRACE] Using fallback OAuth token from Keychain for profile: ' + activeProfile.name, {
tokenFingerprint: getCredentialFingerprint(keychainCreds.token)
});
return keychainCreds.token;
}
// Keychain read also failed
if (keychainCreds.error) {
this.debugLog('[UsageMonitor] Keychain access failed:', keychainCreds.error);
} else {
this.debugLog('[UsageMonitor:TRACE] No token in Keychain for profile: ' + activeProfile.name +
' - user may need to re-authenticate with claude /login');
}
// Mark profile as needing re-authentication since credentials are missing
@@ -863,7 +808,6 @@ export class UsageMonitor extends EventEmitter {
}
// No credential available
this.debugLog('[UsageMonitor:TRACE] No credential available (no API or OAuth profile active)');
return undefined;
}
apps/frontend/src/main/terminal-session-store.ts
-15
View File
@@ -377,23 +377,12 @@ export class TerminalSessionStore {
todaySessions[projectPath] = [];
}
// Debug: Log incoming outputBuffer info
const incomingBufferLen = session.outputBuffer?.length ?? 0;
debugLog('[TerminalSessionStore] Updating session in memory:', session.id,
'incoming outputBuffer:', incomingBufferLen, 'bytes',
'isClaudeMode:', session.isClaudeMode);
// Update existing or add new
const existingIndex = todaySessions[projectPath].findIndex(s => s.id === session.id);
if (existingIndex >= 0) {
// Preserve displayOrder from existing session if not provided in incoming session
// This prevents periodic saves (which don't include displayOrder) from losing tab order
const existingSession = todaySessions[projectPath][existingIndex];
const existingBufferLen = existingSession.outputBuffer?.length ?? 0;
const truncatedLen = session.outputBuffer.slice(-MAX_OUTPUT_BUFFER).length;
debugLog('[TerminalSessionStore] Updating existing session:', session.id,
'existing outputBuffer:', existingBufferLen, 'bytes',
'new outputBuffer (after truncation):', truncatedLen, 'bytes');
todaySessions[projectPath][existingIndex] = {
...session,
@@ -404,10 +393,6 @@ export class TerminalSessionStore {
displayOrder: session.displayOrder ?? existingSession.displayOrder,
};
} else {
const truncatedLen = session.outputBuffer.slice(-MAX_OUTPUT_BUFFER).length;
debugLog('[TerminalSessionStore] Creating new session:', session.id,
'outputBuffer (after truncation):', truncatedLen, 'bytes');
todaySessions[projectPath].push({
...session,
outputBuffer: session.outputBuffer.slice(-MAX_OUTPUT_BUFFER),