Spaces:
Running
Running
Commit
·
f1e4e5b
1
Parent(s):
3139749
fix: address all CodeRabbit Phase 8 review feedback
Browse filesActionable issue:
- Forward additional_properties in run_stream for consistent metadata
Nitpicks addressed:
- Report event now uses actual agent message instead of hardcoded text
- Citation validator: documented in-place mutation, use exact title match
- Log truncation uses constant _MAX_URL_DISPLAY_LENGTH
- Test asserts final_report is stored
- Handle empty lists in markdown (hypotheses, drugs, limitations, refs)
- Edge case: show "Untested" when supported==contradicted==0
- Document citations field in ReportSection as reserved for future use
- src/agents/report_agent.py +5 -1
- src/orchestrator_magentic.py +1 -1
- src/utils/citation_validator.py +18 -6
- src/utils/models.py +29 -12
- tests/unit/agents/test_report_agent.py +2 -0
src/agents/report_agent.py
CHANGED
|
@@ -133,4 +133,8 @@ class ReportAgent(BaseAgent): # type: ignore[misc]
|
|
| 133 |
) -> AsyncIterable[AgentRunResponseUpdate]:
|
| 134 |
"""Streaming wrapper."""
|
| 135 |
result = await self.run(messages, thread=thread, **kwargs)
|
| 136 |
-
yield AgentRunResponseUpdate(
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 133 |
) -> AsyncIterable[AgentRunResponseUpdate]:
|
| 134 |
"""Streaming wrapper."""
|
| 135 |
result = await self.run(messages, thread=thread, **kwargs)
|
| 136 |
+
yield AgentRunResponseUpdate(
|
| 137 |
+
messages=result.messages,
|
| 138 |
+
response_id=result.response_id,
|
| 139 |
+
additional_properties=result.additional_properties,
|
| 140 |
+
)
|
src/orchestrator_magentic.py
CHANGED
|
@@ -266,7 +266,7 @@ The final output should be a complete research report with:
|
|
| 266 |
elif "report" in agent_name.lower():
|
| 267 |
return AgentEvent(
|
| 268 |
type="synthesizing",
|
| 269 |
-
message="Report generated
|
| 270 |
iteration=iteration,
|
| 271 |
)
|
| 272 |
return AgentEvent(
|
|
|
|
| 266 |
elif "report" in agent_name.lower():
|
| 267 |
return AgentEvent(
|
| 268 |
type="synthesizing",
|
| 269 |
+
message=f"Report agent: {_truncate(msg_text)}" if msg_text else "Report generated.",
|
| 270 |
iteration=iteration,
|
| 271 |
)
|
| 272 |
return AgentEvent(
|
src/utils/citation_validator.py
CHANGED
|
@@ -12,22 +12,29 @@ if TYPE_CHECKING:
|
|
| 12 |
|
| 13 |
logger = logging.getLogger(__name__)
|
| 14 |
|
|
|
|
|
|
|
|
|
|
| 15 |
|
| 16 |
def validate_references(report: "ResearchReport", evidence: list["Evidence"]) -> "ResearchReport":
|
| 17 |
"""Ensure all references actually exist in collected evidence.
|
| 18 |
|
| 19 |
CRITICAL: Prevents LLM hallucination of citations.
|
| 20 |
|
|
|
|
|
|
|
|
|
|
|
|
|
| 21 |
Args:
|
| 22 |
-
report: The generated research report
|
| 23 |
evidence: All evidence collected during research
|
| 24 |
|
| 25 |
Returns:
|
| 26 |
-
|
| 27 |
"""
|
| 28 |
# Build set of valid URLs from evidence
|
| 29 |
valid_urls = {e.citation.url for e in evidence}
|
| 30 |
-
# Also check titles (case-insensitive) as fallback
|
| 31 |
valid_titles = {e.citation.title.lower() for e in evidence}
|
| 32 |
|
| 33 |
validated_refs = []
|
|
@@ -40,14 +47,19 @@ def validate_references(report: "ResearchReport", evidence: list["Evidence"]) ->
|
|
| 40 |
# Check if URL matches collected evidence
|
| 41 |
if ref_url in valid_urls:
|
| 42 |
validated_refs.append(ref)
|
| 43 |
-
# Fallback:
|
| 44 |
-
elif ref_title and
|
| 45 |
validated_refs.append(ref)
|
| 46 |
else:
|
| 47 |
removed_count += 1
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 48 |
logger.warning(
|
| 49 |
f"Removed hallucinated reference: '{ref.get('title', 'Unknown')}' "
|
| 50 |
-
f"(URL: {
|
| 51 |
)
|
| 52 |
|
| 53 |
if removed_count > 0:
|
|
|
|
| 12 |
|
| 13 |
logger = logging.getLogger(__name__)
|
| 14 |
|
| 15 |
+
# Max characters to display for URLs in log messages
|
| 16 |
+
_MAX_URL_DISPLAY_LENGTH = 80
|
| 17 |
+
|
| 18 |
|
| 19 |
def validate_references(report: "ResearchReport", evidence: list["Evidence"]) -> "ResearchReport":
|
| 20 |
"""Ensure all references actually exist in collected evidence.
|
| 21 |
|
| 22 |
CRITICAL: Prevents LLM hallucination of citations.
|
| 23 |
|
| 24 |
+
Note:
|
| 25 |
+
This function MUTATES report.references in-place and returns the same
|
| 26 |
+
report object. This is intentional for efficiency.
|
| 27 |
+
|
| 28 |
Args:
|
| 29 |
+
report: The generated research report (will be mutated)
|
| 30 |
evidence: All evidence collected during research
|
| 31 |
|
| 32 |
Returns:
|
| 33 |
+
The same report object with references updated in-place
|
| 34 |
"""
|
| 35 |
# Build set of valid URLs from evidence
|
| 36 |
valid_urls = {e.citation.url for e in evidence}
|
| 37 |
+
# Also check titles (case-insensitive, exact match) as fallback
|
| 38 |
valid_titles = {e.citation.title.lower() for e in evidence}
|
| 39 |
|
| 40 |
validated_refs = []
|
|
|
|
| 47 |
# Check if URL matches collected evidence
|
| 48 |
if ref_url in valid_urls:
|
| 49 |
validated_refs.append(ref)
|
| 50 |
+
# Fallback: exact title match (case-insensitive)
|
| 51 |
+
elif ref_title and ref_title in valid_titles:
|
| 52 |
validated_refs.append(ref)
|
| 53 |
else:
|
| 54 |
removed_count += 1
|
| 55 |
+
# Truncate URL for display
|
| 56 |
+
if len(ref_url) > _MAX_URL_DISPLAY_LENGTH:
|
| 57 |
+
url_display = ref_url[:_MAX_URL_DISPLAY_LENGTH] + "..."
|
| 58 |
+
else:
|
| 59 |
+
url_display = ref_url
|
| 60 |
logger.warning(
|
| 61 |
f"Removed hallucinated reference: '{ref.get('title', 'Unknown')}' "
|
| 62 |
+
f"(URL: {url_display})"
|
| 63 |
)
|
| 64 |
|
| 65 |
if removed_count > 0:
|
src/utils/models.py
CHANGED
|
@@ -177,6 +177,7 @@ class ReportSection(BaseModel):
|
|
| 177 |
|
| 178 |
title: str
|
| 179 |
content: str
|
|
|
|
| 180 |
citations: list[str] = Field(default_factory=list)
|
| 181 |
|
| 182 |
|
|
@@ -224,10 +225,17 @@ class ResearchReport(BaseModel):
|
|
| 224 |
|
| 225 |
# Hypotheses
|
| 226 |
sections.append("## Hypotheses Tested\n")
|
|
|
|
|
|
|
| 227 |
for h in self.hypotheses_tested:
|
| 228 |
supported = h.get("supported", 0)
|
| 229 |
contradicted = h.get("contradicted", 0)
|
| 230 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 231 |
sections.append(
|
| 232 |
f"- **{h.get('mechanism', 'Unknown')}** ({status}): "
|
| 233 |
f"{supported} supporting, {contradicted} contradicting\n"
|
|
@@ -239,26 +247,35 @@ class ResearchReport(BaseModel):
|
|
| 239 |
|
| 240 |
# Drug candidates
|
| 241 |
sections.append("## Drug Candidates\n")
|
| 242 |
-
|
| 243 |
-
|
|
|
|
|
|
|
|
|
|
| 244 |
|
| 245 |
# Limitations
|
| 246 |
sections.append("## Limitations\n")
|
| 247 |
-
|
| 248 |
-
|
|
|
|
|
|
|
|
|
|
| 249 |
|
| 250 |
# Conclusion
|
| 251 |
sections.append(f"## Conclusion\n{self.conclusion}\n")
|
| 252 |
|
| 253 |
# References
|
| 254 |
sections.append("## References\n")
|
| 255 |
-
|
| 256 |
-
|
| 257 |
-
|
| 258 |
-
|
| 259 |
-
|
| 260 |
-
|
| 261 |
-
|
|
|
|
|
|
|
|
|
|
| 262 |
|
| 263 |
# Metadata footer
|
| 264 |
sections.append("\n---\n")
|
|
|
|
| 177 |
|
| 178 |
title: str
|
| 179 |
content: str
|
| 180 |
+
# Reserved for future inline citation tracking within sections
|
| 181 |
citations: list[str] = Field(default_factory=list)
|
| 182 |
|
| 183 |
|
|
|
|
| 225 |
|
| 226 |
# Hypotheses
|
| 227 |
sections.append("## Hypotheses Tested\n")
|
| 228 |
+
if not self.hypotheses_tested:
|
| 229 |
+
sections.append("*No hypotheses tested yet.*\n")
|
| 230 |
for h in self.hypotheses_tested:
|
| 231 |
supported = h.get("supported", 0)
|
| 232 |
contradicted = h.get("contradicted", 0)
|
| 233 |
+
if supported == 0 and contradicted == 0:
|
| 234 |
+
status = "❓ Untested"
|
| 235 |
+
elif supported > contradicted:
|
| 236 |
+
status = "✅ Supported"
|
| 237 |
+
else:
|
| 238 |
+
status = "⚠️ Mixed"
|
| 239 |
sections.append(
|
| 240 |
f"- **{h.get('mechanism', 'Unknown')}** ({status}): "
|
| 241 |
f"{supported} supporting, {contradicted} contradicting\n"
|
|
|
|
| 247 |
|
| 248 |
# Drug candidates
|
| 249 |
sections.append("## Drug Candidates\n")
|
| 250 |
+
if self.drug_candidates:
|
| 251 |
+
for drug in self.drug_candidates:
|
| 252 |
+
sections.append(f"- **{drug}**\n")
|
| 253 |
+
else:
|
| 254 |
+
sections.append("*No drug candidates identified.*\n")
|
| 255 |
|
| 256 |
# Limitations
|
| 257 |
sections.append("## Limitations\n")
|
| 258 |
+
if self.limitations:
|
| 259 |
+
for lim in self.limitations:
|
| 260 |
+
sections.append(f"- {lim}\n")
|
| 261 |
+
else:
|
| 262 |
+
sections.append("*No limitations documented.*\n")
|
| 263 |
|
| 264 |
# Conclusion
|
| 265 |
sections.append(f"## Conclusion\n{self.conclusion}\n")
|
| 266 |
|
| 267 |
# References
|
| 268 |
sections.append("## References\n")
|
| 269 |
+
if self.references:
|
| 270 |
+
for i, ref in enumerate(self.references, 1):
|
| 271 |
+
sections.append(
|
| 272 |
+
f"{i}. {ref.get('authors', 'Unknown')}. "
|
| 273 |
+
f"*{ref.get('title', 'Untitled')}*. "
|
| 274 |
+
f"{ref.get('source', '')} ({ref.get('date', '')}). "
|
| 275 |
+
f"[Link]({ref.get('url', '#')})\n"
|
| 276 |
+
)
|
| 277 |
+
else:
|
| 278 |
+
sections.append("*No references available.*\n")
|
| 279 |
|
| 280 |
# Metadata footer
|
| 281 |
sections.append("\n---\n")
|
tests/unit/agents/test_report_agent.py
CHANGED
|
@@ -109,6 +109,8 @@ async def test_report_agent_generates_report(
|
|
| 109 |
assert "Executive Summary" in response.messages[0].text
|
| 110 |
assert "Methodology" in response.messages[0].text
|
| 111 |
assert "References" in response.messages[0].text
|
|
|
|
|
|
|
| 112 |
|
| 113 |
|
| 114 |
@pytest.mark.asyncio
|
|
|
|
| 109 |
assert "Executive Summary" in response.messages[0].text
|
| 110 |
assert "Methodology" in response.messages[0].text
|
| 111 |
assert "References" in response.messages[0].text
|
| 112 |
+
# Verify report is stored in evidence store
|
| 113 |
+
assert "final_report" in store
|
| 114 |
|
| 115 |
|
| 116 |
@pytest.mark.asyncio
|