Consolidate duplicated patterns in openclaw bridge#81
Conversation
Extract applyOpenClawSessionMetadata (5 call sites), consolidate seq-extraction, replace clone-via-JSON with jsonutil.DeepCloneMap, extract parsedTokenUsage, unify URL normalization pair, and extract supportsFeature helper. ~34 lines removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThese changes refactor the openclaw bridge by extracting helper functions and types to consolidate duplicated logic. Gateway client refactoring introduces helpers for feature matching, sequence number computation, and URL normalization. Manager refactoring extracts helpers for session metadata population and token usage parsing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
bridges/openclaw/manager.go (2)
1376-1412: ReuseparseTokenUsagein the terminal accounting path too.
parseTokenUsagenow centralizes normalized-key parsing, but Lines 1857-1873 still unpack the same fields manually. A smallTotalOrDerived()follow-up would let that block share the same parser and avoid drift.Also applies to: 1428-1437
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bridges/openclaw/manager.go` around lines 1376 - 1412, Replace the duplicated manual unpacking of token fields in the terminal accounting code with the centralized parseTokenUsage function: call parseTokenUsage(usage) to populate a parsedTokenUsage, then assign the fields into the existing MessageMetadata (or use a small helper TotalOrDerived() on parsedTokenUsage if the total needs to be computed when missing). Update both places that manually extract prompt_tokens/completion_tokens/reasoning_tokens/total_tokens to use parseTokenUsage and then set metadata.PromptTokens, metadata.CompletionTokens, metadata.ReasoningTokens and metadata.TotalTokens (or use parsed.TotalOrDerived() if you implement that helper) so the parsing logic is shared with applyUsageToMessageMetadata.
1308-1328: Trim insideapplyOpenClawSessionMetadata.Now that this helper is the single write path for
session_id/session_key/error_text, normalize here too.payload.SessionKeyis still passed through raw at Line 2060, so metadata can drift from the trimmed values the rest of the manager uses.✂️ Suggested cleanup
func applyOpenClawSessionMetadata(m map[string]any, sessionID, sessionKey, errorText string) { + if m == nil { + return + } + sessionID = strings.TrimSpace(sessionID) + sessionKey = strings.TrimSpace(sessionKey) + errorText = strings.TrimSpace(errorText) if sessionID != "" { m["session_id"] = sessionID }Also applies to: 2033-2033, 2060-2060, 2404-2404, 2658-2662
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bridges/openclaw/manager.go` around lines 1308 - 1328, applyOpenClawSessionMetadata currently writes session_id/session_key/error_text without normalizing, which allows untrimmed values to persist; update applyOpenClawSessionMetadata (the function taking m map[string]any, sessionID, sessionKey, errorText string) to trim whitespace from sessionID, sessionKey, and errorText (e.g., strings.TrimSpace) before checking for emptiness and setting m["session_id"], m["session_key"], and m["error_text"] so all call sites can pass raw values and metadata will remain normalized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@bridges/openclaw/manager.go`:
- Around line 1376-1412: Replace the duplicated manual unpacking of token fields
in the terminal accounting code with the centralized parseTokenUsage function:
call parseTokenUsage(usage) to populate a parsedTokenUsage, then assign the
fields into the existing MessageMetadata (or use a small helper TotalOrDerived()
on parsedTokenUsage if the total needs to be computed when missing). Update both
places that manually extract
prompt_tokens/completion_tokens/reasoning_tokens/total_tokens to use
parseTokenUsage and then set metadata.PromptTokens, metadata.CompletionTokens,
metadata.ReasoningTokens and metadata.TotalTokens (or use
parsed.TotalOrDerived() if you implement that helper) so the parsing logic is
shared with applyUsageToMessageMetadata.
- Around line 1308-1328: applyOpenClawSessionMetadata currently writes
session_id/session_key/error_text without normalizing, which allows untrimmed
values to persist; update applyOpenClawSessionMetadata (the function taking m
map[string]any, sessionID, sessionKey, errorText string) to trim whitespace from
sessionID, sessionKey, and errorText (e.g., strings.TrimSpace) before checking
for emptiness and setting m["session_id"], m["session_key"], and m["error_text"]
so all call sites can pass raw values and metadata will remain normalized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf420b95-91b4-423b-be0e-5ae564f1c069
📒 Files selected for processing (2)
bridges/openclaw/gateway_client.gobridges/openclaw/manager.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-agentremote-docker (arm64)
- GitHub Check: build-agentremote-docker (amd64)
- GitHub Check: Lint
- GitHub Check: build-docker
🔇 Additional comments (3)
bridges/openclaw/gateway_client.go (3)
608-635: Nice feature-gating cleanup.
supportsFeaturekeeps the trim/case-insensitive matching logic in one place for both methods and events.
27-27: History sequence/cloning dedupe looks good.Reusing the shared sequence parser and
jsonutil.DeepCloneMapremoves two parallel implementations while preserving the existing fallback behavior in this path.Also applies to: 1161-1175
1371-1405: URL normalization consolidation looks solid.The shared helper keeps the HTTP↔WS scheme translation rules consistent across both entry points.
1 participant
Summary
Test plan
go build ./...passesgo test ./...passesgo vet ./...passes🤖 Generated with Claude Code