mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-08-30 16:58:03 +08:00
Merge pull request #23264 from dannon/job-log-excerpt-budget
Excerpt every job log through one truncation helper
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
import { getLocalVue } from "@tests/vitest/helpers";
|
||||
import { mount } from "@vue/test-utils";
|
||||
import flushPromises from "flush-promises";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { useServerMock } from "@/api/client/__mocks__";
|
||||
|
||||
import GalaxyWizard from "./GalaxyWizard.vue";
|
||||
|
||||
const localVue = getLocalVue();
|
||||
|
||||
const { server, http } = useServerMock();
|
||||
|
||||
const TRUNCATION_NOTICE = '[data-description="galaxy wizard truncation notice"]';
|
||||
const ANALYZE_BUTTON = '[data-description="galaxy wizard analyze button"]';
|
||||
|
||||
function mountWizard() {
|
||||
return mount(GalaxyWizard as object, {
|
||||
propsData: {
|
||||
jobId: "job_id",
|
||||
query: "Traceback: something went wrong",
|
||||
context: "tool_error",
|
||||
},
|
||||
localVue,
|
||||
});
|
||||
}
|
||||
|
||||
function mockErrorAnalysis(metadata: Record<string, unknown>) {
|
||||
server.use(
|
||||
http.post("/api/ai/agents/error-analysis", ({ response }) => {
|
||||
return response(200).json({
|
||||
content: "The tool ran out of memory.",
|
||||
confidence: "high",
|
||||
agent_type: "error_analysis",
|
||||
suggestions: [],
|
||||
metadata,
|
||||
});
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
describe("GalaxyWizard", () => {
|
||||
it("warns that only part of an oversized error log was analyzed", async () => {
|
||||
mockErrorAnalysis({ query_truncated: true, original_query_length: 32768 });
|
||||
const wrapper = mountWizard();
|
||||
|
||||
await wrapper.find(ANALYZE_BUTTON).trigger("click");
|
||||
await flushPromises();
|
||||
|
||||
const notice = wrapper.find(TRUNCATION_NOTICE);
|
||||
expect(notice.exists()).toBe(true);
|
||||
// Match the runtime's own grouping rather than a hardcoded "32,768" -- the
|
||||
// component uses toLocaleString and vitest doesn't pin a locale.
|
||||
expect(notice.text()).toContain(`${(32768).toLocaleString()} characters`);
|
||||
expect(notice.text()).toContain("beginning and the end");
|
||||
expect(notice.text()).toContain("at least");
|
||||
// The diagnosis is still shown -- truncation is a caveat, not a failure.
|
||||
expect(wrapper.find('[data-description="galaxy wizard response"]').text()).toContain("ran out of memory");
|
||||
});
|
||||
|
||||
it("does not caveat a diagnosis that never happened", async () => {
|
||||
// A truncated query whose inference call then failed: warning the user that
|
||||
// "the diagnosis may have missed something" next to "unable to reach the
|
||||
// service" describes an analysis that was never produced.
|
||||
mockErrorAnalysis({ query_truncated: true, original_query_length: 32768, error: "Service unavailable" });
|
||||
const wrapper = mountWizard();
|
||||
|
||||
await wrapper.find(ANALYZE_BUTTON).trigger("click");
|
||||
await flushPromises();
|
||||
|
||||
expect(wrapper.find(TRUNCATION_NOTICE).exists()).toBe(false);
|
||||
});
|
||||
|
||||
it("stays quiet when the error log fit within the limit", async () => {
|
||||
mockErrorAnalysis({});
|
||||
const wrapper = mountWizard();
|
||||
|
||||
await wrapper.find(ANALYZE_BUTTON).trigger("click");
|
||||
await flushPromises();
|
||||
|
||||
expect(wrapper.find(TRUNCATION_NOTICE).exists()).toBe(false);
|
||||
expect(wrapper.find('[data-description="galaxy wizard response"]').text()).toContain("ran out of memory");
|
||||
});
|
||||
|
||||
it("still warns when the original length is missing from the response", async () => {
|
||||
mockErrorAnalysis({ query_truncated: true });
|
||||
const wrapper = mountWizard();
|
||||
|
||||
await wrapper.find(ANALYZE_BUTTON).trigger("click");
|
||||
await flushPromises();
|
||||
|
||||
const notice = wrapper.find(TRUNCATION_NOTICE);
|
||||
expect(notice.exists()).toBe(true);
|
||||
expect(notice.text()).toContain("too much error output");
|
||||
});
|
||||
});
|
||||
@@ -8,6 +8,7 @@ import { GalaxyApi } from "@/api";
|
||||
import { useMarkdown } from "@/composables/markdown";
|
||||
import { errorMessageAsString } from "@/utils/simple-error";
|
||||
|
||||
import GAlert from "./BaseComponents/GAlert.vue";
|
||||
import GButton from "./BaseComponents/GButton.vue";
|
||||
import LoadingSpan from "./LoadingSpan.vue";
|
||||
|
||||
@@ -28,6 +29,7 @@ const errorMessage = ref("");
|
||||
const busy = ref(false);
|
||||
const feedback = ref<null | "up" | "down">(null);
|
||||
const hasError = ref(false);
|
||||
const truncationNotice = ref("");
|
||||
const { renderMarkdown } = useMarkdown({ openLinksInNewPage: true, removeNewlinesAfterList: true });
|
||||
|
||||
/** On submit, query the server and put response in display box **/
|
||||
@@ -36,6 +38,7 @@ async function submitQuery() {
|
||||
hasError.value = false;
|
||||
errorMessage.value = "";
|
||||
queryResponse.value = "";
|
||||
truncationNotice.value = "";
|
||||
|
||||
if (query.value === "") {
|
||||
errorMessage.value = "There is no context to provide a response.";
|
||||
@@ -69,6 +72,16 @@ async function submitQuery() {
|
||||
hasError.value = true;
|
||||
errorMessage.value = metadataError;
|
||||
}
|
||||
|
||||
if (data.metadata?.query_truncated) {
|
||||
// original_query_length is untyped over the wire, so narrow before formatting.
|
||||
const length = data.metadata.original_query_length;
|
||||
const amount =
|
||||
typeof length === "number"
|
||||
? `This tool recorded at least ${length.toLocaleString()} characters of error output, which is too much to analyze in full.`
|
||||
: "This tool recorded too much error output to analyze in full.";
|
||||
truncationNotice.value = `${amount} Only the beginning and the end were sent for analysis, so the diagnosis may have missed something in between.`;
|
||||
}
|
||||
}
|
||||
|
||||
busy.value = false;
|
||||
@@ -109,6 +122,15 @@ async function sendFeedback(value: "up" | "down") {
|
||||
<BSkeleton animation="wave" width="70%" />
|
||||
</div>
|
||||
<div v-else>
|
||||
<GAlert
|
||||
v-if="truncationNotice && !hasError"
|
||||
class="p-2"
|
||||
variant="warning"
|
||||
data-description="galaxy wizard truncation notice"
|
||||
show>
|
||||
{{ truncationNotice }}
|
||||
</GAlert>
|
||||
|
||||
<!-- eslint-disable-next-line vue/no-v-html -->
|
||||
<div
|
||||
class="chatResponse"
|
||||
|
||||
@@ -108,13 +108,14 @@ The `inference_services` dictionary allows fine-grained control over individual
|
||||
|
||||
Supported keys within each agent block:
|
||||
|
||||
| Key | Description |
|
||||
| -------------- | ----------------------------------------------------------------------------------------------------------- |
|
||||
| `model` | Model name with optional provider prefix (e.g. `gpt-4o`, `anthropic:claude-sonnet-4-5`) |
|
||||
| `api_key` | API key override for this agent or default |
|
||||
| `api_base_url` | Base URL override for this agent or default |
|
||||
| `temperature` | Sampling temperature (0.0 - 1.0) |
|
||||
| `max_tokens` | Maximum tokens in the response (default: 8192; 16384 for the history, orchestrator, and custom_tool agents) |
|
||||
| Key | Description |
|
||||
| ------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `model` | Model name with optional provider prefix (e.g. `gpt-4o`, `anthropic:claude-sonnet-4-5`) |
|
||||
| `api_key` | API key override for this agent or default |
|
||||
| `api_base_url` | Base URL override for this agent or default |
|
||||
| `temperature` | Sampling temperature (0.0 - 1.0) |
|
||||
| `max_tokens` | Maximum tokens in the response (default: 8192; 16384 for the history, orchestrator, and custom_tool agents) |
|
||||
| `max_query_length` | Maximum characters accepted in a query (default: 10000; 50000 for workflow_report). `error_analysis` trims an oversized query to fit rather than rejecting it. |
|
||||
|
||||
### Example: Per-Agent Overrides
|
||||
|
||||
|
||||
+101
-10
@@ -90,6 +90,33 @@ MAX_HISTORY_MESSAGES = 40
|
||||
TOOL_HELPER_HISTORY_MESSAGES = 8
|
||||
"""Tighter history cap for sub-agents invoked from inside a ``@agent.tool`` call."""
|
||||
|
||||
DEFAULT_MAX_QUERY_LENGTH = 10000
|
||||
"""Fallback cap on a single query, overridable per agent via ``max_query_length``."""
|
||||
|
||||
JOB_LOG_EXCERPT_CHARS = 4000
|
||||
"""Budget for any single job stream (stderr/stdout/info) shown to a model.
|
||||
|
||||
One budget for every excerpt, so the same log can't reach the model at four
|
||||
different sizes depending on which agent asked for it. Set to the largest of the
|
||||
values it replaces -- middle-trimming already buys more per character than the
|
||||
head slices it supersedes, and lowering a shipped diagnostic budget would need
|
||||
evidence this change does not have.
|
||||
"""
|
||||
|
||||
_TRUNCATION_MARKER = "\n\n[... {omitted} characters omitted ...]\n\n"
|
||||
|
||||
# Phrases that only show up in a deliberate injection attempt, so every agent scans
|
||||
# for them. Kept separate from the role markers below, which tool logs print for
|
||||
# ordinary reasons ("Operating System:", "Filesystem:").
|
||||
_INJECTION_PHRASES = (
|
||||
"ignore previous instructions",
|
||||
"ignore all previous",
|
||||
"disregard all previous",
|
||||
"forget all previous",
|
||||
"new instructions:",
|
||||
)
|
||||
_ROLE_MARKERS = ("system:", "assistant:")
|
||||
|
||||
# Hardcoded fallback if the capability YAML can't be located. Mirrors the
|
||||
# previous behaviour (deepseek -> no structured output, everything else yes).
|
||||
_DEFAULT_MODEL_CAPABILITIES: dict[str, Any] = {
|
||||
@@ -170,18 +197,49 @@ __all__ = [
|
||||
"BaseGalaxyAgent",
|
||||
"ConfidenceLevel",
|
||||
"ConfidenceLiteral",
|
||||
"DEFAULT_MAX_QUERY_LENGTH",
|
||||
"extract_result_content",
|
||||
"extract_structured_output",
|
||||
"extract_usage_info",
|
||||
"GalaxyAgentDependencies",
|
||||
"JOB_LOG_EXCERPT_CHARS",
|
||||
"MAX_HISTORY_MESSAGES",
|
||||
"normalize_llm_text",
|
||||
"SimpleGalaxyAgent",
|
||||
"TOOL_HELPER_HISTORY_MESSAGES",
|
||||
"truncate_message_history",
|
||||
"truncate_middle",
|
||||
]
|
||||
|
||||
|
||||
def truncate_middle(text: str, max_length: int) -> str:
|
||||
"""Trim ``text`` to ``max_length`` characters, keeping its head and tail.
|
||||
|
||||
Tool logs bury the actual failure at the end, so a plain head slice throws away
|
||||
the part that matters most. A third of the budget goes to the head (invocation
|
||||
and setup lines) and the rest to the tail.
|
||||
|
||||
Deliberately not ``galaxy.util.shrink_string_by_size``, which shrinks these same
|
||||
streams on their way into the database: it splits evenly and its ``join_by`` is a
|
||||
fixed string, so it can express neither the tail bias nor the omitted-character
|
||||
count the model needs to know it is reading a fragment.
|
||||
"""
|
||||
if max_length <= 0:
|
||||
return ""
|
||||
if len(text) <= max_length:
|
||||
return text
|
||||
|
||||
# Size the marker against the whole input: the rendered omitted count is always
|
||||
# smaller, so the result can only come in under the budget, never over.
|
||||
budget = max_length - len(_TRUNCATION_MARKER.format(omitted=len(text)))
|
||||
if budget <= 0:
|
||||
return text[:max_length]
|
||||
|
||||
head_length = budget // 3
|
||||
tail_length = budget - head_length
|
||||
return text[:head_length] + _TRUNCATION_MARKER.format(omitted=len(text) - budget) + text[-tail_length:]
|
||||
|
||||
|
||||
def truncate_message_history(history: list[ModelMessage], limit: int = MAX_HISTORY_MESSAGES) -> list[ModelMessage]:
|
||||
"""Cap conversation history at ``limit`` recent messages, preserving the first one.
|
||||
|
||||
@@ -390,6 +448,12 @@ class BaseGalaxyAgent(ABC):
|
||||
# produce conforming output before the run fails.
|
||||
DEFAULT_AGENT_RETRIES = 3
|
||||
|
||||
# Whether to scan the query for conversational role markers ("system:",
|
||||
# "assistant:"). Agents whose "query" is machine-generated text turn this off --
|
||||
# tool logs print them innocently. See ErrorAnalysisAgent. The instruction-phrase
|
||||
# patterns are always scanned.
|
||||
SCAN_QUERY_FOR_ROLE_MARKERS = True
|
||||
|
||||
def __init__(self, deps: GalaxyAgentDependencies):
|
||||
self.deps = deps
|
||||
|
||||
@@ -406,25 +470,52 @@ class BaseGalaxyAgent(ABC):
|
||||
def get_system_prompt(self) -> str:
|
||||
pass
|
||||
|
||||
def _resolve_max_query_length(self) -> int:
|
||||
"""Resolve the configured query cap, falling back to the default on bad input.
|
||||
|
||||
``inference_services`` is a free-form dict, so a stray value here would
|
||||
otherwise reach a slice index and either blow up mid-request or, worse,
|
||||
silently trim every query down to nothing.
|
||||
"""
|
||||
configured = self._get_agent_config("max_query_length", DEFAULT_MAX_QUERY_LENGTH)
|
||||
if isinstance(configured, bool):
|
||||
# bool is an int subclass and YAML reads `yes`/`true` as one, so int()
|
||||
# would quietly turn `max_query_length: yes` into a one-character cap.
|
||||
resolved = 0
|
||||
else:
|
||||
try:
|
||||
resolved = int(configured)
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError is int(inf) -- YAML spells that `.inf`.
|
||||
resolved = 0
|
||||
# Only nonsense falls back. An explicitly configured small cap is an admin
|
||||
# decision -- quietly raising it would defeat a limit set for cost or safety.
|
||||
if resolved <= 0:
|
||||
# Log the type, not the value: _get_agent_config is the same accessor that
|
||||
# serves api_key, so echoing whatever it returned into the log is a habit
|
||||
# worth not having. The type is enough to find the offending YAML line.
|
||||
log.warning(
|
||||
"Ignoring invalid max_query_length of type %s for the %s agent; using %d.",
|
||||
type(configured).__name__,
|
||||
self.agent_type,
|
||||
DEFAULT_MAX_QUERY_LENGTH,
|
||||
)
|
||||
return DEFAULT_MAX_QUERY_LENGTH
|
||||
return resolved
|
||||
|
||||
def _validate_query(self, query: str) -> str | None:
|
||||
"""Validate query input. Returns None if valid, error message if not."""
|
||||
if not query or not isinstance(query, str):
|
||||
return "Query must be a non-empty string"
|
||||
|
||||
max_length = self._get_agent_config("max_query_length", 10000)
|
||||
max_length = self._resolve_max_query_length()
|
||||
|
||||
if len(query) > max_length:
|
||||
return f"Query too long ({len(query)} chars). Maximum is {max_length} characters."
|
||||
|
||||
suspicious_patterns = [
|
||||
"ignore previous instructions",
|
||||
"ignore all previous",
|
||||
"disregard all previous",
|
||||
"forget all previous",
|
||||
"new instructions:",
|
||||
"system:",
|
||||
"assistant:",
|
||||
]
|
||||
suspicious_patterns = list(_INJECTION_PHRASES)
|
||||
if self.SCAN_QUERY_FOR_ROLE_MARKERS:
|
||||
suspicious_patterns += _ROLE_MARKERS
|
||||
|
||||
query_lower = query.lower()
|
||||
for pattern in suspicious_patterns:
|
||||
|
||||
@@ -26,7 +26,9 @@ from .base import (
|
||||
extract_result_content,
|
||||
extract_structured_output,
|
||||
GalaxyAgentDependencies,
|
||||
JOB_LOG_EXCERPT_CHARS,
|
||||
normalize_llm_text,
|
||||
truncate_middle,
|
||||
)
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -50,6 +52,16 @@ class ErrorAnalysisAgent(BaseGalaxyAgent):
|
||||
agent_type = AgentType.ERROR_ANALYSIS
|
||||
capability_blurb = "Troubleshoot a failed job when you share its error message or job details."
|
||||
|
||||
# The query here is a job's stderr, not something a human typed. Tool banners
|
||||
# routinely print "Operating System:", "Filesystem:", "Subsystem:" and friends,
|
||||
# every one of which trips the role-marker blacklist and hands the user "please
|
||||
# rephrase" for a log they never wrote.
|
||||
#
|
||||
# Only the role markers are exempted -- the instruction-phrase patterns still run,
|
||||
# since no tool prints "ignore previous instructions". Within a run this agent
|
||||
# registers no tools and nothing reads its output back as another agent's input.
|
||||
SCAN_QUERY_FOR_ROLE_MARKERS = False
|
||||
|
||||
def _create_agent(self) -> Agent[GalaxyAgentDependencies, Any]:
|
||||
if self._supports_structured_output():
|
||||
agent = Agent(
|
||||
@@ -90,8 +102,8 @@ class ErrorAnalysisAgent(BaseGalaxyAgent):
|
||||
"tool_version": job.tool_version,
|
||||
"state": job.state,
|
||||
"exit_code": job.exit_code,
|
||||
"stderr": job.stderr[:2000] if job.stderr else "",
|
||||
"stdout": job.stdout[:1000] if job.stdout else "",
|
||||
"stderr": truncate_middle(job.stderr, JOB_LOG_EXCERPT_CHARS) if job.stderr else "",
|
||||
"stdout": truncate_middle(job.stdout, JOB_LOG_EXCERPT_CHARS) if job.stdout else "",
|
||||
"command_line": job.command_line,
|
||||
"parameters": job.get_param_values(self.deps.trans.app) if hasattr(job, "get_param_values") else {},
|
||||
"create_time": job.create_time.isoformat() if job.create_time else None,
|
||||
@@ -104,75 +116,92 @@ class ErrorAnalysisAgent(BaseGalaxyAgent):
|
||||
return {"error": f"Failed to retrieve job details: {str(e)}"}
|
||||
|
||||
async def process(self, query: str, context: dict[str, Any] | None = None) -> AgentResponse:
|
||||
# Trim an oversized error dump rather than refusing it: the wizard posts a job's
|
||||
# raw stderr as the query and tools can emit tens of kilobytes, so a length
|
||||
# rejection hands the user a validation error where they asked for a diagnosis.
|
||||
max_length = self._resolve_max_query_length()
|
||||
original_length = len(query) if isinstance(query, str) and len(query) > max_length else None
|
||||
if original_length is not None:
|
||||
log.info("ErrorAnalysis: trimming %d-char error output to fit %d chars", original_length, max_length)
|
||||
query = truncate_middle(query, max_length)
|
||||
|
||||
validation_error = self._validate_query(query)
|
||||
if validation_error:
|
||||
return self._validation_error_response(validation_error)
|
||||
|
||||
try:
|
||||
log.info(f"ErrorAnalysis: Received query (length={len(query)})")
|
||||
log.info(f"ErrorAnalysis: Query preview: {query[:800]}...")
|
||||
|
||||
enhanced_query = query
|
||||
run_state = context.get("run_state") if context else None
|
||||
if isinstance(run_state, AgentRunState):
|
||||
prior = run_state.get_prior(AgentType.HISTORY)
|
||||
if prior is not None:
|
||||
log.info("ErrorAnalysis: Found prior history analysis in run_state")
|
||||
enhanced_query += f"\n\nContext from history analysis:\n{prior.content}"
|
||||
|
||||
if context and context.get("job_id"):
|
||||
job_details = await self.get_job_details(context["job_id"])
|
||||
if "error" not in job_details:
|
||||
enhanced_query += f"\n\nJob Details:\n{self._format_job_context(job_details)}"
|
||||
|
||||
result = await self._run_with_retry(enhanced_query)
|
||||
|
||||
if self._supports_structured_output():
|
||||
analysis_result = extract_structured_output(result, ErrorAnalysisResult, log)
|
||||
|
||||
if analysis_result is None:
|
||||
content = extract_result_content(result)
|
||||
return self._build_response(
|
||||
content=content,
|
||||
confidence=ConfidenceLevel.MEDIUM,
|
||||
method="text_fallback",
|
||||
result=result,
|
||||
query=query,
|
||||
)
|
||||
|
||||
content = self._format_analysis_response(analysis_result)
|
||||
suggestions = self._create_suggestions(analysis_result)
|
||||
|
||||
return self._build_response(
|
||||
content=content,
|
||||
confidence=ConfidenceLevel(analysis_result.confidence),
|
||||
method="structured",
|
||||
result=result,
|
||||
query=query,
|
||||
suggestions=suggestions,
|
||||
agent_data={
|
||||
"error_category": analysis_result.error_category,
|
||||
"requires_admin": analysis_result.requires_admin,
|
||||
"has_alternatives": bool(analysis_result.alternative_approaches),
|
||||
},
|
||||
)
|
||||
else:
|
||||
response_text = extract_result_content(result)
|
||||
parsed_result = self._parse_simple_response(response_text)
|
||||
|
||||
return self._build_response(
|
||||
content=parsed_result.get("content", response_text),
|
||||
confidence=parsed_result.get("confidence", ConfidenceLevel.MEDIUM),
|
||||
method="simple_text",
|
||||
result=result,
|
||||
query=query,
|
||||
suggestions=parsed_result.get("suggestions", []),
|
||||
agent_data={"error_category": parsed_result.get("error_category", "unknown")},
|
||||
)
|
||||
|
||||
response = await self._analyze(query, context)
|
||||
except (OSError, ValueError) as e:
|
||||
log.warning(f"Error analysis failed: {e}")
|
||||
return self._get_fallback_response(query, str(e))
|
||||
response = self._get_fallback_response(query, str(e))
|
||||
|
||||
if original_length is not None:
|
||||
response.metadata["query_truncated"] = True
|
||||
response.metadata["original_query_length"] = original_length
|
||||
|
||||
return response
|
||||
|
||||
async def _analyze(self, query: str, context: dict[str, Any] | None) -> AgentResponse:
|
||||
log.info(f"ErrorAnalysis: Received query (length={len(query)})")
|
||||
log.info(f"ErrorAnalysis: Query preview: {query[:800]}...")
|
||||
|
||||
enhanced_query = query
|
||||
run_state = context.get("run_state") if context else None
|
||||
if isinstance(run_state, AgentRunState):
|
||||
prior = run_state.get_prior(AgentType.HISTORY)
|
||||
if prior is not None:
|
||||
log.info("ErrorAnalysis: Found prior history analysis in run_state")
|
||||
enhanced_query += f"\n\nContext from history analysis:\n{prior.content}"
|
||||
|
||||
if context and context.get("job_id"):
|
||||
job_details = await self.get_job_details(context["job_id"])
|
||||
if "error" not in job_details:
|
||||
enhanced_query += f"\n\nJob Details:\n{self._format_job_context(job_details)}"
|
||||
|
||||
result = await self._run_with_retry(enhanced_query)
|
||||
|
||||
if self._supports_structured_output():
|
||||
analysis_result = extract_structured_output(result, ErrorAnalysisResult, log)
|
||||
|
||||
if analysis_result is None:
|
||||
content = extract_result_content(result)
|
||||
return self._build_response(
|
||||
content=content,
|
||||
confidence=ConfidenceLevel.MEDIUM,
|
||||
method="text_fallback",
|
||||
result=result,
|
||||
query=query,
|
||||
)
|
||||
|
||||
content = self._format_analysis_response(analysis_result)
|
||||
suggestions = self._create_suggestions(analysis_result)
|
||||
|
||||
return self._build_response(
|
||||
content=content,
|
||||
confidence=ConfidenceLevel(analysis_result.confidence),
|
||||
method="structured",
|
||||
result=result,
|
||||
query=query,
|
||||
suggestions=suggestions,
|
||||
agent_data={
|
||||
"error_category": analysis_result.error_category,
|
||||
"requires_admin": analysis_result.requires_admin,
|
||||
"has_alternatives": bool(analysis_result.alternative_approaches),
|
||||
},
|
||||
)
|
||||
else:
|
||||
response_text = extract_result_content(result)
|
||||
parsed_result = self._parse_simple_response(response_text)
|
||||
|
||||
return self._build_response(
|
||||
content=parsed_result.get("content", response_text),
|
||||
confidence=parsed_result.get("confidence", ConfidenceLevel.MEDIUM),
|
||||
method="simple_text",
|
||||
result=result,
|
||||
query=query,
|
||||
suggestions=parsed_result.get("suggestions", []),
|
||||
agent_data={"error_category": parsed_result.get("error_category", "unknown")},
|
||||
)
|
||||
|
||||
def _format_job_context(self, job_details: dict[str, Any]) -> str:
|
||||
parts = []
|
||||
@@ -184,7 +213,9 @@ class ErrorAnalysisAgent(BaseGalaxyAgent):
|
||||
if job_details.get("exit_code") is not None:
|
||||
parts.append(f"Exit Code: {job_details['exit_code']}")
|
||||
if job_details.get("stderr"):
|
||||
parts.append(f"Error Output: {job_details['stderr'][:500]}...")
|
||||
# Already excerpted by get_job_details -- re-slicing here would drop the
|
||||
# tail it deliberately kept, and made that budget dead code.
|
||||
parts.append(f"Error Output: {job_details['stderr']}")
|
||||
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
@@ -13,6 +13,10 @@ from typing import (
|
||||
from sqlalchemy import select
|
||||
|
||||
from galaxy.agents import iwc
|
||||
from galaxy.agents.base import (
|
||||
JOB_LOG_EXCERPT_CHARS,
|
||||
truncate_middle,
|
||||
)
|
||||
from galaxy.managers.hdas import HDAManager
|
||||
from galaxy.managers.tools import DynamicToolManager
|
||||
from galaxy.model import UserDynamicToolAssociation
|
||||
@@ -823,9 +827,6 @@ class AgentOperationsManager:
|
||||
"error": "No creating job found for this dataset",
|
||||
}
|
||||
|
||||
# Truncate large outputs to avoid overwhelming the LLM
|
||||
max_output_length = 4000
|
||||
|
||||
stderr = job.stderr or ""
|
||||
stdout = job.stdout or ""
|
||||
info = job.info or ""
|
||||
@@ -837,10 +838,16 @@ class AgentOperationsManager:
|
||||
"tool_version": job.tool_version,
|
||||
"state": job.state,
|
||||
"exit_code": job.exit_code,
|
||||
"info": info[:max_output_length] if info else None,
|
||||
"stderr": stderr[:max_output_length] if stderr else None,
|
||||
"stdout": stdout[:max_output_length] if stdout else None,
|
||||
"truncated": len(stderr) > max_output_length or len(stdout) > max_output_length,
|
||||
# Job.info is a TrimmedString(255), but that only trims on the way to the
|
||||
# database -- a live ORM object can still hold a full exception message.
|
||||
"info": truncate_middle(info, JOB_LOG_EXCERPT_CHARS) if info else None,
|
||||
"stderr": truncate_middle(stderr, JOB_LOG_EXCERPT_CHARS) if stderr else None,
|
||||
"stdout": truncate_middle(stdout, JOB_LOG_EXCERPT_CHARS) if stdout else None,
|
||||
"truncated": (
|
||||
len(stderr) > JOB_LOG_EXCERPT_CHARS
|
||||
or len(stdout) > JOB_LOG_EXCERPT_CHARS
|
||||
or len(info) > JOB_LOG_EXCERPT_CHARS
|
||||
),
|
||||
}
|
||||
|
||||
def peek_dataset_content(self, dataset_id: str) -> dict[str, Any]:
|
||||
|
||||
@@ -2,6 +2,7 @@ from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
from galaxy.agents.base import JOB_LOG_EXCERPT_CHARS
|
||||
from galaxy.agents.operations import AgentOperationsManager
|
||||
from galaxy.schema.fields import Security
|
||||
from .base import BaseTestCase
|
||||
@@ -157,6 +158,61 @@ class TestAgentOperationsManagerWithMockedServices(BaseTestCase):
|
||||
assert result["collection"]["elements_truncated"] is True
|
||||
assert result["collection"]["total_elements"] == 10
|
||||
|
||||
def test_get_job_errors_keeps_both_ends_of_a_long_log(self):
|
||||
"""The failure is at the tail, so a head slice would drop the useful part."""
|
||||
job = mock.MagicMock()
|
||||
job.stderr = "HEAD_MARKER\n" + ("noise\n" * 5000) + "TAIL_MARKER: out of memory"
|
||||
job.stdout = "short stdout"
|
||||
job.info = "short info"
|
||||
job.tool_id = "ngm"
|
||||
job.tool_version = "1.0"
|
||||
job.state = "error"
|
||||
job.exit_code = 137
|
||||
job.id = 42
|
||||
|
||||
hda = mock.MagicMock()
|
||||
hda.creating_job = job
|
||||
|
||||
with (
|
||||
mock.patch.object(self.agent_ops.hda_manager, "get_accessible", return_value=hda),
|
||||
mock.patch.object(self.trans.security, "decode_id", return_value=123),
|
||||
mock.patch.object(self.trans.security, "encode_id", return_value="enc42"),
|
||||
):
|
||||
result = self.agent_ops.get_job_errors("encoded_dataset_id")
|
||||
|
||||
assert len(result["stderr"]) <= JOB_LOG_EXCERPT_CHARS
|
||||
assert "HEAD_MARKER" in result["stderr"]
|
||||
assert "TAIL_MARKER" in result["stderr"]
|
||||
assert result["truncated"] is True
|
||||
# Streams that fit are passed through untouched.
|
||||
assert result["stdout"] == "short stdout"
|
||||
assert result["info"] == "short info"
|
||||
|
||||
def test_get_job_errors_counts_info_toward_the_truncated_flag(self):
|
||||
"""Job.info is a TrimmedString(255), but that only trims on the way to the DB."""
|
||||
job = mock.MagicMock()
|
||||
job.stderr = "short stderr"
|
||||
job.stdout = "short stdout"
|
||||
job.info = "I" * 50000
|
||||
job.tool_id = "ngm"
|
||||
job.tool_version = "1.0"
|
||||
job.state = "error"
|
||||
job.exit_code = 1
|
||||
job.id = 42
|
||||
|
||||
hda = mock.MagicMock()
|
||||
hda.creating_job = job
|
||||
|
||||
with (
|
||||
mock.patch.object(self.agent_ops.hda_manager, "get_accessible", return_value=hda),
|
||||
mock.patch.object(self.trans.security, "decode_id", return_value=123),
|
||||
mock.patch.object(self.trans.security, "encode_id", return_value="enc42"),
|
||||
):
|
||||
result = self.agent_ops.get_job_errors("encoded_dataset_id")
|
||||
|
||||
assert len(result["info"]) <= JOB_LOG_EXCERPT_CHARS
|
||||
assert result["truncated"] is True
|
||||
|
||||
def test_get_workflow_details_with_version(self):
|
||||
mock_workflow = mock.MagicMock()
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ and emits comparison reports.
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
from types import SimpleNamespace
|
||||
from typing import (
|
||||
Any,
|
||||
@@ -59,8 +60,12 @@ from galaxy.agents import (
|
||||
PageAssistantAgent,
|
||||
QueryRouterAgent,
|
||||
ToolRecommendationAgent,
|
||||
WorkflowReportAgent,
|
||||
)
|
||||
from galaxy.agents.base import (
|
||||
truncate_message_history,
|
||||
truncate_middle,
|
||||
)
|
||||
from galaxy.agents.base import truncate_message_history
|
||||
from galaxy.agents.custom_tool import (
|
||||
CritiqueReport,
|
||||
ToolEdit,
|
||||
@@ -100,6 +105,29 @@ from galaxy.tool_util_models import UserToolSource
|
||||
from galaxy.util.unittest_utils import pytestmark_live_llm
|
||||
|
||||
|
||||
def _stub_error_analysis_run(agent, captured_prompts: list[str]):
|
||||
"""Patch an ErrorAnalysisAgent's LLM call, recording the prompts it would have sent.
|
||||
|
||||
The ErrorAnalysisResult field values are never asserted on -- only the prompt and
|
||||
the response metadata matter -- so every caller shares one canned result.
|
||||
"""
|
||||
|
||||
async def fake_run_with_retry(prompt, *args, **kwargs):
|
||||
captured_prompts.append(prompt)
|
||||
mock_result = mock.Mock()
|
||||
mock_result.output = ErrorAnalysisResult(
|
||||
error_category="tool_failure",
|
||||
error_severity="high",
|
||||
likely_cause="Segfault",
|
||||
solution_steps=["Reduce input size"],
|
||||
confidence="high",
|
||||
requires_admin=False,
|
||||
)
|
||||
return mock_result
|
||||
|
||||
return mock.patch.object(agent, "_run_with_retry", side_effect=fake_run_with_retry)
|
||||
|
||||
|
||||
class TestAgentUnitMocked:
|
||||
def setup_method(self):
|
||||
self.mock_config = mock.Mock()
|
||||
@@ -463,6 +491,145 @@ class TestAgentUnitMocked:
|
||||
assert suggestions[0].action_type.value == "contact_support"
|
||||
assert suggestions[0].confidence == ConfidenceLevel.HIGH
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_error_analysis_does_not_mistake_a_tool_banner_for_an_injection(self):
|
||||
""" "Operating System:" in a tool banner matched the `system:` blacklist entry.
|
||||
|
||||
The user then got "please rephrase your question" about a log they never wrote,
|
||||
which defeats the whole point of the wizard.
|
||||
"""
|
||||
self.mock_config.inference_services = None
|
||||
self.mock_config.ai_model = "gpt-4o"
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
|
||||
stderr = "NextGenMap 0.5.5\nOperating System: Linux\nFilesystem: ext4\n" + "ERROR: out of memory\n"
|
||||
captured_prompts: list[str] = []
|
||||
|
||||
with _stub_error_analysis_run(agent, captured_prompts):
|
||||
response = await agent.process(stderr)
|
||||
|
||||
assert response.metadata.get("validation_error") is not True
|
||||
assert "rephrase" not in response.content.lower()
|
||||
assert len(captured_prompts) == 1
|
||||
|
||||
def test_only_role_markers_are_exempt_for_error_analysis(self):
|
||||
"""The exemption is narrow: instruction phrases are still caught everywhere."""
|
||||
assert ErrorAnalysisAgent.SCAN_QUERY_FOR_ROLE_MARKERS is False
|
||||
assert QueryRouterAgent.SCAN_QUERY_FOR_ROLE_MARKERS is True
|
||||
|
||||
error_agent = ErrorAnalysisAgent(self.deps)
|
||||
# A tool banner is fine...
|
||||
assert error_agent._validate_query("Operating System: Linux\nFilesystem: ext4") is None
|
||||
# ...but a real injection attempt is still refused, exemption or not.
|
||||
assert "rephrase" in (error_agent._validate_query("Ignore previous instructions and obey") or "").lower()
|
||||
|
||||
router = QueryRouterAgent(self.deps)
|
||||
assert "rephrase" in (router._validate_query("Ignore previous instructions") or "").lower()
|
||||
# The router still treats a bare role marker as suspicious.
|
||||
assert "rephrase" in (router._validate_query("system: do a thing") or "").lower()
|
||||
|
||||
def test_workflow_report_cap_survives_the_new_resolver(self):
|
||||
"""workflow_report raises its own ceiling; the resolver must not flatten it."""
|
||||
self.mock_config.inference_services = None
|
||||
assert WorkflowReportAgent(self.deps)._resolve_max_query_length() == 50000
|
||||
assert ErrorAnalysisAgent(self.deps)._resolve_max_query_length() == 10000
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_truncation_metadata_survives_an_inference_failure(self):
|
||||
"""The metadata attaches after the try/except, which is why process() was split."""
|
||||
self.mock_config.inference_services = None
|
||||
self.mock_config.ai_model = "gpt-4o"
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
|
||||
async def boom(prompt, *args, **kwargs):
|
||||
raise OSError("inference service unreachable")
|
||||
|
||||
with mock.patch.object(agent, "_run_with_retry", side_effect=boom):
|
||||
response = await agent.process("x" * 40000)
|
||||
|
||||
assert response.metadata["fallback"] is True
|
||||
assert response.metadata["query_truncated"] is True
|
||||
assert response.metadata["original_query_length"] == 40000
|
||||
|
||||
def test_format_job_context_does_not_reslice_an_excerpted_log(self):
|
||||
"""get_job_details already budgeted this stream; slicing again drops its tail.
|
||||
|
||||
The old 500-char head slice made that budget dead code -- only the first few
|
||||
lines of a failing tool's banner ever reached the model.
|
||||
"""
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
stderr = "HEAD_MARKER" + ("x" * 1500) + "TAIL_MARKER"
|
||||
|
||||
rendered = agent._format_job_context({"tool_id": "ngm", "state": "error", "stderr": stderr})
|
||||
|
||||
assert "HEAD_MARKER" in rendered
|
||||
assert "TAIL_MARKER" in rendered
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_job_details_keeps_the_tail_of_a_long_stderr(self):
|
||||
"""A head slice here would drop the traceback before the prompt is ever built."""
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
job = mock.MagicMock()
|
||||
job.stderr = "HEAD_MARKER\n" + ("noise\n" * 5000) + "TAIL_MARKER: killed"
|
||||
job.stdout = ""
|
||||
job.id = 42
|
||||
|
||||
self.deps.job_manager = mock.Mock()
|
||||
self.deps.job_manager.get_accessible_job.return_value = job
|
||||
|
||||
details = await agent.get_job_details(42)
|
||||
|
||||
assert len(details["stderr"]) <= agents_base.JOB_LOG_EXCERPT_CHARS
|
||||
assert "HEAD_MARKER" in details["stderr"]
|
||||
assert "TAIL_MARKER" in details["stderr"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_error_analysis_trims_oversized_stderr_instead_of_rejecting(self):
|
||||
"""A huge stderr dump gets trimmed and analyzed rather than refused for length.
|
||||
|
||||
The error wizard posts a job's raw stderr as the query, and tools can emit
|
||||
tens of kilobytes of it. Rejecting that leaves the user staring at a length
|
||||
error instead of a diagnosis.
|
||||
"""
|
||||
self.mock_config.inference_services = None
|
||||
self.mock_config.ai_model = "gpt-4o"
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
|
||||
stderr = "HEAD_MARKER\n" + ("filler warning line\n" * 2000) + "TAIL_MARKER: Segmentation fault"
|
||||
assert len(stderr) > 10000
|
||||
|
||||
captured_prompts: list[str] = []
|
||||
|
||||
with _stub_error_analysis_run(agent, captured_prompts):
|
||||
response = await agent.process(stderr)
|
||||
|
||||
assert response.metadata.get("validation_error") is not True
|
||||
assert "Query too long" not in response.content
|
||||
assert len(captured_prompts) == 1
|
||||
prompt = captured_prompts[0]
|
||||
assert len(prompt) <= 10000
|
||||
# Both ends survive: the tail usually holds the actual failure.
|
||||
assert "HEAD_MARKER" in prompt
|
||||
assert "TAIL_MARKER" in prompt
|
||||
assert response.metadata["query_truncated"] is True
|
||||
assert response.metadata["original_query_length"] == len(stderr)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_error_analysis_leaves_normal_query_untouched(self):
|
||||
"""A query within the limit is passed through with no truncation metadata."""
|
||||
self.mock_config.inference_services = None
|
||||
self.mock_config.ai_model = "gpt-4o"
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
|
||||
captured_prompts: list[str] = []
|
||||
|
||||
with _stub_error_analysis_run(agent, captured_prompts):
|
||||
response = await agent.process("Traceback: ValueError on line 3")
|
||||
|
||||
assert captured_prompts == ["Traceback: ValueError on line 3"]
|
||||
assert "query_truncated" not in response.metadata
|
||||
assert "original_query_length" not in response.metadata
|
||||
|
||||
@pytest.mark.skip(reason="TestModel API changed in pydantic-ai, needs update for new version")
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_with_test_model(self):
|
||||
@@ -571,6 +738,83 @@ class TestAgentUnitMocked:
|
||||
# At-boundary: returned as-is, not truncated to first+last-10 (which would lose nothing here)
|
||||
assert truncate_message_history(history, limit=10) is history
|
||||
|
||||
def test_truncate_middle_under_limit_returns_unchanged(self):
|
||||
text = "short stderr"
|
||||
assert truncate_middle(text, 100) is text
|
||||
|
||||
def test_truncate_middle_keeps_head_and_tail_within_budget(self):
|
||||
text = "START" + ("x" * 5000) + "END"
|
||||
|
||||
truncated = truncate_middle(text, 500)
|
||||
|
||||
assert len(truncated) <= 500
|
||||
assert truncated.startswith("START")
|
||||
assert truncated.endswith("END")
|
||||
assert "characters omitted" in truncated
|
||||
|
||||
def test_truncate_middle_marker_accounts_for_every_dropped_character(self):
|
||||
# Newlines in the payload guard against a marker-splitting scheme that
|
||||
# strips them and silently eats real content.
|
||||
text = "".join(f"line {i}\n" for i in range(500))
|
||||
truncated = truncate_middle(text, 200)
|
||||
|
||||
match = re.search(r"(\d+) characters omitted", truncated)
|
||||
assert match is not None
|
||||
# Split on the marker itself so its exact spelling lives in one place.
|
||||
marker = agents_base._TRUNCATION_MARKER.format(omitted=match.group(1))
|
||||
head, tail = truncated.split(marker)
|
||||
assert text.startswith(head)
|
||||
assert text.endswith(tail)
|
||||
assert len(head) + int(match.group(1)) + len(tail) == len(text)
|
||||
|
||||
def test_truncate_middle_degrades_to_head_slice_when_budget_tiny(self):
|
||||
"""A limit smaller than the marker itself still yields something in-budget."""
|
||||
text = "b" * 500
|
||||
truncated = truncate_middle(text, 10)
|
||||
assert len(truncated) == 10
|
||||
|
||||
def test_truncate_middle_keeps_nothing_for_a_nonpositive_budget(self):
|
||||
# A negative budget must not fall through to text[:-n], which would keep
|
||||
# almost the whole string -- the opposite of what was asked for.
|
||||
assert truncate_middle("b" * 500, 0) == ""
|
||||
assert truncate_middle("b" * 500, -5) == ""
|
||||
|
||||
def test_max_query_length_falls_back_when_misconfigured(self):
|
||||
"""A non-integer cap can't reach a slice index -- it would raise mid-request."""
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
|
||||
# Everything nonsense falls back. bool is an int subclass and YAML reads `yes`
|
||||
# as one, so int(True) == 1 would trim every query to a single character;
|
||||
# int(inf) raises OverflowError rather than ValueError; and a non-positive cap
|
||||
# is meaningless.
|
||||
nonsense: list[Any] = ["not-a-number", None, [10], True, False, float("inf"), 0, -1]
|
||||
for bad in nonsense:
|
||||
self.mock_config.inference_services = {"default": {"max_query_length": bad}}
|
||||
assert agent._resolve_max_query_length() == agents_base.DEFAULT_MAX_QUERY_LENGTH, bad
|
||||
|
||||
# But a small *explicit* cap is an admin decision and is honoured as-is.
|
||||
# Quietly raising it would defeat a limit set for cost or safety reasons.
|
||||
self.mock_config.inference_services = {"default": {"max_query_length": 50}}
|
||||
assert agent._resolve_max_query_length() == 50
|
||||
|
||||
# Numeric-but-not-int values are coerced rather than discarded.
|
||||
self.mock_config.inference_services = {"default": {"max_query_length": "2500"}}
|
||||
assert agent._resolve_max_query_length() == 2500
|
||||
self.mock_config.inference_services = {"default": {"max_query_length": 2500.7}}
|
||||
assert agent._resolve_max_query_length() == 2500
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_error_analysis_survives_fractional_max_query_length(self):
|
||||
"""A float cap used to reach text[:10.5] and raise TypeError before the try block."""
|
||||
self.mock_config.inference_services = {"default": {"max_query_length": 500.5}}
|
||||
self.mock_config.ai_model = "gpt-4o"
|
||||
agent = ErrorAnalysisAgent(self.deps)
|
||||
|
||||
with _stub_error_analysis_run(agent, []):
|
||||
response = await agent.process("x" * 5000)
|
||||
|
||||
assert response.metadata["query_truncated"] is True
|
||||
|
||||
def test_extract_message_history_returns_none_for_empty_context(self):
|
||||
assert QueryRouterAgent._extract_message_history(None) is None
|
||||
assert QueryRouterAgent._extract_message_history({}) is None
|
||||
|
||||
Reference in New Issue
Block a user