IssueTriage fixes
This commit is contained in:
@@ -1,3 +1,8 @@
|
||||
### 2026-09-21
|
||||
1. Now comments are directed to team members with care of the language they can read
|
||||
2. Comments for context are filtered from TriageAgent to avoid hallucination
|
||||
3. Fixed missing ambiguous comment
|
||||
|
||||
### 2026-09-20
|
||||
1. Each agent has a *Model* for both input and output
|
||||
2. Implemented base class for Model that provide abstract render_dbg function (allows print debug info in console) and to_prompt_text that allows to summarize a model for prompting
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
from dataclasses import dataclass
|
||||
|
||||
from agents.issue_triage.context_builder import YouTrackContextBuilder
|
||||
@@ -170,6 +171,21 @@ Do NOT produce a plan. Do NOT suggest new agents in this step.
|
||||
"missing_capabilities": ["<deduped tags from objections>"],
|
||||
"questions": ["..."] // only if status = INSUFFICIENT_INFO
|
||||
}
|
||||
|
||||
Interpretation, aspect, why it matters, suggested_question and assumptions_planner_may_make should be written in project language: {{COMMENT_LANGUAGE}}
|
||||
missing_capabilities - in english
|
||||
|
||||
## ROUTING
|
||||
|
||||
Every finding MUST include a `directed_to` field. It must be one of the
|
||||
`nick` values listed under Team members, without the leading `@`.
|
||||
|
||||
Choose the member who owns the aspect the question is about:
|
||||
{{TEAM}}
|
||||
|
||||
If no team member clearly owns the question, use the Default responder
|
||||
nick. Never leave `directed_to` empty. Never invent a nick that is not
|
||||
in the roster.
|
||||
"""
|
||||
|
||||
AMBIGUITY_PROMPT = """
|
||||
@@ -224,6 +240,20 @@ valid, common, and correct answer.
|
||||
],
|
||||
"questions": ["<deduped suggested_question for blocking findings>"]
|
||||
}
|
||||
|
||||
Interpretation, aspect, why it matters, suggested_question and assumptions_planner_may_make should be written in project language: {{COMMENT_LANGUAGE}}
|
||||
|
||||
## ROUTING
|
||||
|
||||
Every finding MUST include a `directed_to` field. It must be one of the
|
||||
`nick` values listed under Team members, without the leading `@`.
|
||||
|
||||
Choose the member who owns the aspect the question is about:
|
||||
{{TEAM}}
|
||||
|
||||
If no team member clearly owns the question, use the Default responder
|
||||
nick. Never leave `directed_to` empty. Never invent a nick that is not
|
||||
in the roster.
|
||||
"""
|
||||
|
||||
SEQUENCE_PROMPT = """\
|
||||
@@ -545,10 +575,17 @@ class IssueTriageAgent:
|
||||
def audit_capability(
|
||||
self, issue: IssueContext
|
||||
) -> CapabilityResult:
|
||||
raw = self.llm.chat_with_schema(
|
||||
(CAPABILITY_PROMPT
|
||||
cloned_issue = issue
|
||||
cloned_issue.comments = [c for c in cloned_issue.comments if c.get("author") != os.getenv("YOUTRACK_TRIAGE_AUTHOR")]
|
||||
prompt = (CAPABILITY_PROMPT
|
||||
.replace("{{AGENTS}}", self.agent_registry.to_prompt_text())
|
||||
.replace("{{ISSUE}}", issue.to_prompt_text())),
|
||||
.replace("{{COMMENT_LANGUAGE}}", issue.project.project_language if issue.project else "english")
|
||||
.replace("{{ISSUE}}", cloned_issue.to_prompt_text())
|
||||
.replace("{{TEAM}}", "\n".join([f"- `{m.nick}` — {m.role}" for m in issue.project.team] if issue.project else []))
|
||||
)
|
||||
logger.debug(prompt)
|
||||
raw = self.llm.chat_with_schema(
|
||||
prompt,
|
||||
{
|
||||
"type": "json_schema",
|
||||
"json_schema": {
|
||||
@@ -563,15 +600,21 @@ class IssueTriageAgent:
|
||||
def audit_ambiguity(
|
||||
self, issue: IssueContext
|
||||
) -> AmbiguityResult:
|
||||
cloned_issue = issue
|
||||
cloned_issue.comments = [c for c in cloned_issue.comments if
|
||||
c.get("author") != os.getenv("YOUTRACK_TRIAGE_AUTHOR")]
|
||||
prompt = (
|
||||
AMBIGUITY_PROMPT
|
||||
.replace("{{ISSUE}}", issue.to_prompt_text())
|
||||
.replace("{{ISSUE}}", cloned_issue.to_prompt_text())
|
||||
.replace("{{AGENTS}}", self.agent_registry.to_prompt_text())
|
||||
.replace("{{COMMENT_LANGUAGE}}", issue.project.project_language if issue.project else "english")
|
||||
.replace(
|
||||
"{{PROJECT}}",
|
||||
issue.project.to_prompt_text() if issue.project else "",
|
||||
)
|
||||
.replace("{{TEAM}}", "\n".join([f"- `{m.nick}` — {m.role}" for m in issue.project.team] if issue.project else []))
|
||||
)
|
||||
logger.debug(prompt)
|
||||
raw = self.llm.chat_with_schema(
|
||||
prompt,
|
||||
{
|
||||
@@ -613,9 +656,9 @@ class IssueTriageAgent:
|
||||
.replace("{{ASSUMPTIONS}}", "\n".join(f"- {a}" for a in assumptions) if assumptions else "(none — you are free to make low-stakes defaults)")
|
||||
.replace("{{PROJECT}}", issue.project.to_prompt_text() if issue.project else "")
|
||||
)
|
||||
raw = self.llm.client.chat(
|
||||
raw = self.llm.chat_with_schema(
|
||||
prompt,
|
||||
response_format={
|
||||
{
|
||||
"type": "json_schema",
|
||||
"json_schema": {
|
||||
"name": "TaskPlan",
|
||||
|
||||
@@ -56,6 +56,9 @@ class YouTrackContextBuilder:
|
||||
project_key = issue_id.split("-", 1)[0]
|
||||
project = load_project(project_key)
|
||||
|
||||
logger.debug(f"Lookup for project")
|
||||
logger.debug(project.model_dump_json())
|
||||
|
||||
return IssueContext(
|
||||
issue_id=_first(issue, "idReadable", "id") or issue_id,
|
||||
title=_first(issue, "summary", "title", "name", default=""),
|
||||
@@ -65,7 +68,7 @@ class YouTrackContextBuilder:
|
||||
"id": c.get("url"),
|
||||
"author": c.get("author") or "unknown",
|
||||
"body": c.get("text") or c.get("body") or "",
|
||||
"created_at": str(c.get("created") or c.get("created_at") or ""),
|
||||
"created_at": str(c.get("created") or c.get("createdAt") or ""),
|
||||
}
|
||||
for c in comments
|
||||
],
|
||||
|
||||
@@ -1,17 +1,17 @@
|
||||
from collections.abc import Iterator
|
||||
|
||||
from agents.issue_triage.models import Step
|
||||
from agents.issue_triage.models import Step, Plan
|
||||
from contracts.Task import Task
|
||||
from contracts.TaskPlan import TaskPlan
|
||||
|
||||
|
||||
def has_cycle(steps: list[Task]) -> bool:
|
||||
def has_cycle(steps: list[Step]) -> bool:
|
||||
"""
|
||||
Return True if the dependency graph contains a cycle.
|
||||
Ignores depends_on entries that reference unknown step_ids
|
||||
(those are caught by a separate validator).
|
||||
"""
|
||||
by_id = {s.id: s for s in steps}
|
||||
by_id = {s.step_id: s for s in steps}
|
||||
|
||||
WHITE, GRAY, BLACK = 0, 1, 2
|
||||
color: dict[str, int] = {sid: WHITE for sid in by_id}
|
||||
@@ -82,7 +82,7 @@ GLOBAL_INPUTS: frozenset[str] = frozenset({
|
||||
})
|
||||
|
||||
|
||||
def longest_path(steps: list[Task]) -> list[str]:
|
||||
def longest_path(steps: list[Step]) -> list[str]:
|
||||
"""
|
||||
Return the step_ids on the critical path (by estimate_p50).
|
||||
Assumes acyclic. Returns the empty list for empty input.
|
||||
@@ -90,7 +90,7 @@ def longest_path(steps: list[Task]) -> list[str]:
|
||||
if not steps:
|
||||
return []
|
||||
|
||||
by_id = {s.id: s for s in steps}
|
||||
by_id = {s.step_id: s for s in steps}
|
||||
|
||||
# topological order (Kahn's algorithm)
|
||||
indeg: dict[str, int] = {sid: 0 for sid in by_id}
|
||||
@@ -99,8 +99,8 @@ def longest_path(steps: list[Task]) -> list[str]:
|
||||
for dep in s.depends_on:
|
||||
if dep not in by_id:
|
||||
continue
|
||||
indeg[s.id] += 1
|
||||
children[dep].append(s.id)
|
||||
indeg[s.step_id] += 1
|
||||
children[dep].append(s.step_id)
|
||||
|
||||
queue = [sid for sid, d in indeg.items() if d == 0]
|
||||
topo: list[str] = []
|
||||
@@ -147,7 +147,7 @@ def longest_path(steps: list[Task]) -> list[str]:
|
||||
path.reverse()
|
||||
return path
|
||||
|
||||
def path_steps(path: list[str], plan: TaskPlan) -> list[Task]:
|
||||
def path_steps(path: list[str], plan: Plan) -> list[Step]:
|
||||
"""Resolve a list of step_ids to their Step objects, in path order."""
|
||||
by_id = {s.id: s for s in plan.tasks}
|
||||
by_id = {s.step_id: s for s in plan.steps}
|
||||
return [by_id[sid] for sid in path if sid in by_id]
|
||||
@@ -1,9 +1,12 @@
|
||||
import json
|
||||
from dataclasses import dataclass, field
|
||||
import logging
|
||||
from dataclasses import dataclass, field, asdict
|
||||
from typing import Literal
|
||||
|
||||
from pydantic import BaseModel, Field, ValidationError, ConfigDict
|
||||
|
||||
logger = logging.getLogger("models")
|
||||
logger.setLevel(logging.DEBUG)
|
||||
|
||||
class Objection(BaseModel):
|
||||
model_config = ConfigDict(extra="forbid")
|
||||
@@ -33,6 +36,7 @@ class AmbiguityFinding(BaseModel):
|
||||
interpretation_b: str
|
||||
why_it_matters: str
|
||||
suggested_question: str
|
||||
directed_to: str
|
||||
blocking: bool
|
||||
|
||||
|
||||
@@ -165,6 +169,7 @@ def parse_ambiguity_result(raw: str | dict) -> AmbiguityResult:
|
||||
why_it_matters=f.why_it_matters,
|
||||
suggested_question=f.suggested_question,
|
||||
blocking=f.blocking,
|
||||
directed_to=f.directed_to,
|
||||
)
|
||||
for f in model.findings
|
||||
],
|
||||
@@ -207,8 +212,8 @@ def parse_task_plan(raw: str | dict) -> Plan:
|
||||
],
|
||||
parallel_groups=[list(g) for g in model.parallel_groups],
|
||||
critical_path=list(model.critical_path),
|
||||
total_p50=model.total_estimate_minutes.p50,
|
||||
total_p90=model.total_estimate_minutes.p90,
|
||||
total_p50=model.total_p50,
|
||||
total_p90=model.total_p90,
|
||||
assumptions=list(model.assumptions),
|
||||
open_questions=list(model.open_questions),
|
||||
missing_capabilities=list(model.missing_capabilities),
|
||||
@@ -226,6 +231,7 @@ def _check_task_plan_semantics(model: Plan) -> None:
|
||||
raise ValueError("status=INSUFFICIENT_CAPABILITY but steps is non-empty")
|
||||
elif model.status == "INSUFFICIENT_INFO":
|
||||
if not model.open_questions:
|
||||
logger.debug(model.model_dump_json())
|
||||
raise ValueError("status=INSUFFICIENT_INFO but open_questions is empty")
|
||||
if model.steps:
|
||||
raise ValueError("status=INSUFFICIENT_INFO but steps is non-empty")
|
||||
@@ -4,30 +4,30 @@ from dataclasses import dataclass, asdict
|
||||
from agents.issue_triage.helpers import has_cycle, longest_path, path_steps
|
||||
from agents.issue_triage.marker import TriageMarker
|
||||
from agents.issue_triage.merge import Action, merge, MergeDecision
|
||||
from agents.issue_triage.models import CapabilityResult, AmbiguityResult
|
||||
from agents.issue_triage.models import CapabilityResult, AmbiguityResult, Plan
|
||||
from agents.registry import AgentRegistry
|
||||
from contracts.IssueContext import IssueContext
|
||||
from contracts.TaskPlan import TaskPlan
|
||||
|
||||
|
||||
def validate_plan(plan: TaskPlan, registry):
|
||||
def validate_plan(plan: Plan, registry):
|
||||
errors = []
|
||||
|
||||
# 1. All agent_ids exist
|
||||
for step in plan.tasks:
|
||||
if step.agent not in registry and step.agent != "human":
|
||||
errors.append(f"{step.id}: unknown agent {step.agent}")
|
||||
for step in plan.steps:
|
||||
if step.agent_id not in registry and step.agent_id != "human":
|
||||
errors.append(f"{step.step_id}: unknown agent {step.agent_id}")
|
||||
|
||||
# 2. depends_on references valid, earlier-declared step_ids
|
||||
seen = set()
|
||||
for step in plan.tasks:
|
||||
for step in plan.steps:
|
||||
for dep in step.depends_on:
|
||||
if dep not in seen:
|
||||
errors.append(f"{step.id}: forward or unknown dep {dep}")
|
||||
seen.add(step.id)
|
||||
errors.append(f"{step.step_id}: forward or unknown dep {dep}")
|
||||
seen.add(step.step_id)
|
||||
|
||||
# 3. Acyclicity (topological sort)
|
||||
if has_cycle(plan.tasks):
|
||||
if has_cycle(plan.steps):
|
||||
errors.append("dependency cycle detected")
|
||||
|
||||
# 4. Dataflow: inputs must be produced by an ancestor
|
||||
@@ -39,19 +39,19 @@ def validate_plan(plan: TaskPlan, registry):
|
||||
# errors.append(f"{step['step_id']}: input '{inp}' has no producer")
|
||||
|
||||
# 5. Estimates sane
|
||||
for step in plan.tasks:
|
||||
e = step.estimate_minutes
|
||||
if e["p90"] < e["p50"]:
|
||||
errors.append(f"{step.id}: p90 < p50")
|
||||
for step in plan.steps:
|
||||
# if e["p90"] < e["p50"]:
|
||||
if step.estimate_p90 < step.estimate_p50:
|
||||
errors.append(f"{step.step_id}: p90 < p50")
|
||||
|
||||
# 6. Critical path matches depends_on
|
||||
computed = longest_path(plan.tasks)
|
||||
computed = longest_path(plan.steps)
|
||||
if set(computed) != set(plan.critical_path):
|
||||
errors.append("critical_path mismatch")
|
||||
|
||||
# 7. Totals recompute, don't trust
|
||||
recomputed_p50 = sum(s.estimate_minutes["p50"] for s in path_steps(computed, plan))
|
||||
if recomputed_p50 != plan.total_estimate_minutes["p50"]:
|
||||
recomputed_p50 = sum(s.estimate_p50 for s in path_steps(computed, plan))
|
||||
if recomputed_p50 != plan.total_p50:
|
||||
errors.append("total p50 mismatch")
|
||||
|
||||
return errors
|
||||
@@ -92,6 +92,22 @@ def render_questions_comment(
|
||||
for q in decision.questions:
|
||||
lines.append(f"- {q}")
|
||||
|
||||
if amb.status == "AMBIGUOUS" and amb.blocking_findings():
|
||||
for f in amb.blocking_findings():
|
||||
lines += [
|
||||
f"### {f.aspect}",
|
||||
"",
|
||||
f"- **Reading A:** {f.interpretation_a}",
|
||||
f"- **Reading B:** {f.interpretation_b}",
|
||||
f"- **Why it matters:** {f.why_it_matters}",
|
||||
f"- **Question:** {"@" + f.directed_to if f.directed_to not in f.suggested_question else ""} {f.suggested_question}",
|
||||
"",
|
||||
]
|
||||
else:
|
||||
# capability-driven questions — flat list is fine
|
||||
for q in decision.questions:
|
||||
lines.append(f"- {q}")
|
||||
|
||||
# if the capability pass also flagged something, surface it — the
|
||||
# question may resolve it (e.g. "which DB?" could unblock db:migration)
|
||||
if cap.status == "GAP" and cap.missing_capabilities:
|
||||
@@ -109,13 +125,9 @@ def render_questions_comment(
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def plan_to_dict(plan: TaskPlan) -> dict:
|
||||
return asdict(plan)
|
||||
|
||||
|
||||
def render_validation_failure(
|
||||
marker: TriageMarker,
|
||||
plan: TaskPlan,
|
||||
plan: Plan,
|
||||
errors: list[str],
|
||||
) -> str:
|
||||
lines = [
|
||||
@@ -136,7 +148,7 @@ def render_validation_failure(
|
||||
"### Rejected plan (raw)",
|
||||
"",
|
||||
"```json",
|
||||
json.dumps(plan_to_dict(plan), indent=2),
|
||||
json.dumps(plan.model_dump(mode="json"), indent=2),
|
||||
"```",
|
||||
"",
|
||||
"_This comment was posted automatically. Do not edit the marker "
|
||||
@@ -145,7 +157,7 @@ def render_validation_failure(
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def render_plan_comment(marker, plan: TaskPlan, assumptions, questions) -> str:
|
||||
def render_plan_comment(marker, plan: Plan, assumptions, questions) -> str:
|
||||
lines = [
|
||||
marker.render(),
|
||||
f"**Triage: PLANNED** — {plan.summary}",
|
||||
@@ -155,10 +167,10 @@ def render_plan_comment(marker, plan: TaskPlan, assumptions, questions) -> str:
|
||||
"### Steps",
|
||||
"",
|
||||
]
|
||||
for s in plan.tasks:
|
||||
for s in plan.steps:
|
||||
deps = f" (after {', '.join(s.depends_on)})" if s.depends_on else ""
|
||||
lines.append(
|
||||
f"- `{s.id}` **{s.agent}**{deps}: {s.acceptance_criteria} "
|
||||
f"- `{s.step_id}` **{s.agent_id}**{deps}: {s.verification} "
|
||||
f"[{s.estimate_p50}–{s.estimate_p90}m, {s.risk}]"
|
||||
)
|
||||
if assumptions:
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
from dataclasses import field, dataclass
|
||||
|
||||
from pydantic import BaseModel
|
||||
|
||||
@dataclass
|
||||
class ProjectContext:
|
||||
|
||||
class TeamMemberModel(BaseModel):
|
||||
role: str
|
||||
nick: str
|
||||
|
||||
class ProjectContext(BaseModel):
|
||||
"""Static, per-project knowledge injected into every decomposition."""
|
||||
project_key: str # e.g. "ARCH"
|
||||
language: str = ""
|
||||
@@ -15,6 +20,9 @@ class ProjectContext:
|
||||
package_manager: str = "" # "uv", "poetry", "npm"
|
||||
auth: str = "" # "JWT via fastapi-users"
|
||||
deployment: str = "" # "Docker Compose on Hetzner"
|
||||
project_language: str = "english"
|
||||
default_responder: str | None = None
|
||||
team: list[TeamMemberModel] = field(default_factory=list)
|
||||
conventions: list[str] = field(default_factory=list) # free-form notes
|
||||
extra: dict[str, str] = field(default_factory=dict) # anything else
|
||||
|
||||
@@ -31,6 +39,7 @@ class ProjectContext:
|
||||
("Package manager", self.package_manager),
|
||||
("Auth", self.auth),
|
||||
("Deployment", self.deployment),
|
||||
("Comment language", self.project_language),
|
||||
]
|
||||
for label, value in fields:
|
||||
if value:
|
||||
@@ -42,4 +51,15 @@ class ProjectContext:
|
||||
lines.append("")
|
||||
lines.append("Conventions:")
|
||||
lines.extend(f"- {c}" for c in self.conventions)
|
||||
|
||||
if self.team:
|
||||
lines.append("**Team members** (address questions using the exact `nick`):")
|
||||
for m in self.team:
|
||||
lines.append(f"- `{m.nick}` — {m.role}")
|
||||
if self.default_responder:
|
||||
lines.append(
|
||||
f"**Default responder** (use this nick when no team member "
|
||||
f"clearly owns the question): `{self.default_responder}`"
|
||||
)
|
||||
lines.append("")
|
||||
return "\n".join(lines)
|
||||
Reference in New Issue
Block a user