feat(docs comments): resolve which tab a comment belongs to in one fetch - #965
Conversation
`docs comments list` gave no hint which tab a comment belongs to, and `docs comments locate` answers that for one comment per invocation, each paying its own documents.get. Annotating a 30-comment doc cost 30 fetches. Add opt-in `--locate` and `--tab` that share a single documents.get(includeTabsContent=true) across every comment, reusing the matching path `docs comments locate` already uses. Without the new flags the command is unchanged and still needs only the Drive scope. `matches` always spans every tab so callers can detect an ambiguous quote; `--tab` keeps a comment when any match falls in the target tab, which drops orphans and unquoted comments. closes openclaw#961
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 12:10 AM ET / 04:10 UTC. ClawSweeper reviewWhat this changesAdds opt-in Merge readinessKeep open for normal merge: the maintainer explicitly adopted Option A, and the current head has sufficient redacted signed-binary proof for the opt-in behavior. No blocking patch defect was found. Priority: P3 Review scores
Verification
How this fits togetherThe Docs comments command normally lists file-scoped Drive comments. With the new flags, it obtains one Google Docs document view, matches each comment’s quoted text across tabs, and returns enriched JSON or table output. flowchart LR
A[CLI flags] --> B[Drive comment list]
B --> C{Locate or tab requested?}
C -->|No| D[Existing Drive-only output]
C -->|Yes| E[One Google Docs fetch]
E --> F[Resolve quotes across tabs]
F --> G[Filtered JSON or table output]
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Land the approved strictly opt-in batch resolver once the normal required checks complete. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction; the maintainer’s redacted signed-binary run directly demonstrates the new real command behavior after the fix. Is this the best way to solve the issue? Yes. The approved list-command surface reuses the established quote-matching behavior, preserves the default Drive-only path, and avoids one Docs fetch per comment. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e32b1f73a73a. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (16 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Maintainer decision: adopt Option A ( Current-head signed-binary proof ( The real Full local @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
|
Landed in Maintainer work before landing:
Proof:
Thanks @ryo-touch for the unusually thorough proposal, tests, and original live validation. |
|
@steipete Thank you for reviewing and merging this pr. I am looking forward to release! |
2 participants
closes #961
What
Adds two opt-in flags to
gog docs comments list:--locateattaches a per-commentlocationobject (matches,orphaned), resolved from a singledocuments.get(includeTabsContent=true)shared by every comment.--tab <title|id>implies--locateand keeps comments with at least one match in that tab. Resolved through the existingresolveTabArgpath, so the hidden deprecated--tab-idworks like it does on the sibling commands.Today
docs comments locateanswers this for one comment per invocation, each paying its own document fetch — annotating a 23-comment doc costs 23 fetches.{ "docId": "…", "tab": { "id": "t.abc", "title": "Planning" }, // only with --tab "comments": [ { "id": "AAA…", "quotedFileContent": {…}, "location": { "matches": [{"startIndex": 1, "endIndex": 19, "paragraphIndex": 0, "tabId": "t.abc"}], "orphaned": false } } ], "nextPageToken": "" }Design notes
t.Fatalto keep it that way.matchesalways spans every tab, never truncated to the target tab, so a quote appearing in two tabs is detectable by the caller.--tabpasses a comment when any match lands in the tab.--tabdrops anything that resolves to no tab — orphaned quotes and comments with no quoted text (e.g. document-level ones).--locatealone reports them withorphaned: truerather than dropping them.list --locatedeliberately does not inheritlocate'sexitCodeOrphaned(11); orphaned is per-item state here. Only--fail-empty(3) and the usual auth codes apply.--taband no--all, filtering can empty a page, so the command walks pages until one yields a match — mirroring the existing scan-for-open loop inlistDriveComments, and reusing the one already-fetched document (with a seen-token guard against a repeatingnextPageToken).driveCommentWithLocationembeds*drive.Commentand overridesMarshalJSON: the generated type declaresMarshalJSONon a value receiver, so the promoted method would silently droplocation. Same pattern aseventWithCalendarincalendar_list.go; there's a unit test that fails if the override is removed.Option A vs B
The issue asked which shape you'd prefer and I didn't want to sit on the patch, so this is Option A. The batch resolution lives behind
docsCommentLocatorin its own file, so Option B (docs comments locate --all) would only need a different command layer — no change to the resolver. Happy to reshape it, or to close this if you'd rather own the design.Testing
make cigreen.--tabfiltering and implied--locate, cross-tab matches preserved, page scanning,--fail-empty, unknown tab, tabless documents, plain-table columns, and theMarshalJSONregression guard.mainby running the same fixture (replies, truncation, nil entries) through both implementations.--locateresolved all 23 in one fetch (21 located across 5 tabs, 2 orphaned, 2 matching in more than one tab), and everymatchesarray was identical to runningdocs comments locateper comment.--tabby ID and by (Japanese) title returned the same 5 comments.