Show ACP tool arguments in progress summaries - #243
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughTool summaries now retain the first extracted input payload, compute its JSON size, and include formatted arguments. Extraction supports multiple input fields. Argument output uses compact or fallback rendering and truncates long values. ChangesTool summary formatting
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@connector-support/acp/tool_summary.go`:
- Around line 338-341: Update the truncation logic around rendered in
connector-support/acp/tool_summary.go lines 338-341 to reserve the rune length
of the “… [truncated]” marker before slicing, keeping the complete output within
maxToolArgumentChars. Update connector-support/acp/tool_summary_test.go lines
312-317 to remove the 1,100-rune allowance and assert that the text after
“Arguments: ” is at most maxToolArgumentChars runes.
- Around line 307-312: The formatToolCompletionSummaryWithInput function
currently uses only the nil check to decide whether to include Arguments, which
makes explicit JSON null inputs indistinguishable from absent inputs. Add an
input-presence flag parameter to the function to track whether the rawInput
field was explicitly set, then update the condition around the Arguments append
to check the presence flag rather than just the nil check. When the input field
is present (the flag is true), always render "Arguments:" including the case
where the value itself is null, preserving the JSON null-versus-absent
distinction per the jsonpresence coding guidelines.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b7253d40-918b-4f34-8460-c97a8d9e8510
📒 Files selected for processing (2)
connector-support/acp/tool_summary.goconnector-support/acp/tool_summary_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: For server, CLI, worker, or network Go code, ensurecontext.Contextis propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely.
Do not make accidental public API breaks in Go code: underpkg/**or anywhere exported Go identifiers are changed, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.
**/*.go: Implement and describe Go behavior only; explicitly label Python-era or future behavior.
The core owns orchestration, routing, failover, and B2BUA continuity; provider semantics remain in adapters/plugins.
Core code must not import provider SDKs or concrete plugins.
Use canonical protocol adapters instead of pairwise protocol translators.
Streaming is primary; non-streaming APIs must collect the canonical stream.
Do not transparently retry or fail over after the first downstream content event.
Fail explicitly on capability mismatches; never silently drop required semantics.
Place request and response mutation behind hooks or extensions, not core branching.
Use explicit construction and registration; do not use DI containers, reflection registries, globals, or Go nativepluginin v1.
Use specs for new features, breaking or architecture changes, protocol/plugin contract changes, routing semantic changes, or unclear requirements; use direct code for small fixes, documentation, narrow tests, and trivial maintenance.
If an active Kiro spec is in scope, do not code before approvedrequirements.mdanddesign.mdare recorded inspec.json.
Prefer the standard library and add dependencies only when they reduce complexity or risk.
Define small interfaces where consumed; constructors should return concrete types unless exposing stable SDK or plugin contracts.
Every I/O boundary must takecontext.Context; never store contexts in structs.
Own goroutines, channels, and cancellation ex...
Files:
connector-support/acp/tool_summary_test.goconnector-support/acp/tool_summary.go
⚙️ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
connector-support/acp/tool_summary_test.goconnector-support/acp/tool_summary.go
**/*
📄 CodeRabbit inference engine (Custom checks)
Do not introduce hardcoded credentials, API keys, tokens, private keys, passwords, production secrets, or sensitive internal URLs.
**/*: Do not add changelog,_Updated,_Reason, timestamp, or history footers to steering or agent instructions.
Use TDD by default: write the test or interface before the implementation.
Prefer the smallest correct diff and avoid speculative abstractions.
Never claim success without direct verification evidence.
Preserve user-authored changes and never use destructive git commands unless explicitly requested.
Do not work onmain; create a suitably named local worktree and fix/spec/feat branch based onmainfor each change.
Use forward-slash git pathspecs on Windows.
Reports must state changed files and verification run, and plainly state skipped tests or uncertainty; mention architectural trade-offs only when relevant.
Files:
connector-support/acp/tool_summary_test.goconnector-support/acp/tool_summary.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use focused tests with
go test -run TestName ./path/to/pkg; use the project’s prescribed unit, quality, parity, QA, race, and fuzz commands according to change scope.
Files:
connector-support/acp/tool_summary_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.
Files:
connector-support/acp/tool_summary_test.go
connector-support/**
⚙️ CodeRabbit configuration file
connector-support/**: Review shared connector support for secure defaults in process management, TLS, and credential propagation.
Files:
connector-support/acp/tool_summary_test.goconnector-support/acp/tool_summary.go
🧠 Learnings (1)
📚 Learning: 2026-07-01T22:57:42.953Z
Learnt from: matdev83
Repo: matdev83/go-llm-interactive-proxy PR: 101
File: pkg/lipsdk/scope/context.go:0-0
Timestamp: 2026-07-01T22:57:42.953Z
Learning: In this repository, when defining unexported Go `context` key constants of type `ctxKey int` (e.g., `const ( kFoo ctxKey = iota + N )`), preserve the `iota + <offset>` pattern and keep the existing `<offset>` values rather than simplifying to plain `iota`. These per-package offsets are part of the repo-wide convention to avoid key collisions across packages, and each such constant set should include a short explanatory comment (for example: “offset avoids collision with other packages' context keys”).
Applied to files:
connector-support/acp/tool_summary_test.goconnector-support/acp/tool_summary.go
🔇 Additional comments (2)
connector-support/acp/tool_summary.go (1)
4-4: LGTM!Also applies to: 22-22, 95-99, 231-238, 301-305, 323-337
connector-support/acp/tool_summary_test.go (1)
22-22: LGTM!Also applies to: 55-57
d207cb3 to
f2a579d
Compare
1 participant
What changed
Why
The AGY ACP wrapper already sends tool parameters through rawInput. Go LIP retained only their byte count, so clients saw tool names and sizes without the paths, queries, or command arguments needed to understand progress.
User impact
ACP progress summaries now show useful tool arguments while remaining bounded for large payloads.
Verification