fix(go-agent): Added PubMed component support - #16817
Conversation
📝 WalkthroughWalkthroughPubMed retrieval now uses esearch followed by efetch XML, with configurable node defaults for ChangesPubMed integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Canvas
participant pubMedComponent
participant PubMedTool
participant NCBI
Canvas->>pubMedComponent: Invoke query
pubMedComponent->>PubMedTool: InvokableRun query
PubMedTool->>NCBI: esearch query
NCBI-->>PubMedTool: PMID list
PubMedTool->>NCBI: efetch PMIDs as XML
NCBI-->>PubMedTool: XML articles
PubMedTool-->>pubMedComponent: JSON envelope
pubMedComponent-->>Canvas: formalized_content and json
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agent/component/universe_a_wrappers.go`:
- Around line 1728-1737: In the error-handling branch of PubMedTool, avoid
returning a nil error when decoded lacks a usable "_ERROR" value: use
decoded["_ERROR"] when present and non-empty, otherwise fall back to
err.Error(). Preserve the partial response while ensuring the returned error
always contains the upstream failure; update the logic around the len(decoded)
check accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45c11e92-a78a-40f7-8ebd-d0dd3f845f65
📒 Files selected for processing (7)
internal/agent/component/fixture_stubs.gointernal/agent/component/pubmed_component_test.gointernal/agent/component/universe_a_wrappers.gointernal/agent/component/verify_p1_test.gointernal/agent/tool/pubmed.gointernal/agent/tool/pubmed_test.gointernal/agent/tool/registry.go
| if err != nil { | ||
| if len(decoded) > 0 { | ||
| return map[string]any{ | ||
| "formalized_content": "", | ||
| "json": results, | ||
| "_ERROR": decoded["_ERROR"], | ||
| }, nil | ||
| } | ||
| return nil, fmt.Errorf("canvas: PubMed: %w", err) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Error can be swallowed when _ERROR is absent from decoded output.
When err != nil and len(decoded) > 0 but the decoded envelope lacks a usable _ERROR field, the code returns decoded["_ERROR"] (which is nil when the key is absent) and a nil error. The upstream error is lost. While the current PubMedTool always sets _ERROR on error, this path silently drops errors from any invoker that returns partial JSON without _ERROR.
🛡️ Proposed fix: fall back to err.Error() when _ERROR is absent
if err != nil {
if len(decoded) > 0 {
+ errMsg, _ := decoded["_ERROR"].(string)
+ if strings.TrimSpace(errMsg) == "" {
+ errMsg = err.Error()
+ }
return map[string]any{
"formalized_content": "",
"json": results,
- "_ERROR": decoded["_ERROR"],
+ "_ERROR": errMsg,
}, nil
}
return nil, fmt.Errorf("canvas: PubMed: %w", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if err != nil { | |
| if len(decoded) > 0 { | |
| return map[string]any{ | |
| "formalized_content": "", | |
| "json": results, | |
| "_ERROR": decoded["_ERROR"], | |
| }, nil | |
| } | |
| return nil, fmt.Errorf("canvas: PubMed: %w", err) | |
| } | |
| if err != nil { | |
| if len(decoded) > 0 { | |
| errMsg, _ := decoded["_ERROR"].(string) | |
| if strings.TrimSpace(errMsg) == "" { | |
| errMsg = err.Error() | |
| } | |
| return map[string]any{ | |
| "formalized_content": "", | |
| "json": results, | |
| "_ERROR": errMsg, | |
| }, nil | |
| } | |
| return nil, fmt.Errorf("canvas: PubMed: %w", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/agent/component/universe_a_wrappers.go` around lines 1728 - 1737, In
the error-handling branch of PubMedTool, avoid returning a nil error when
decoded lacks a usable "_ERROR" value: use decoded["_ERROR"] when present and
non-empty, otherwise fall back to err.Error(). Preserve the partial response
while ensuring the returned error always contains the upstream failure; update
the logic around the len(decoded) check accordingly.
Summary
Tests
bash build.sh --test ./internal/agent/component/...bash build.sh --test ./internal/agent/tool/...