Skip to content

feat(docs): list pending suggestions#896

Open
steipete wants to merge 3 commits into
mainfrom
codex/docs-suggestions-list
Open

feat(docs): list pending suggestions#896
steipete wants to merge 3 commits into
mainfrom
codex/docs-suggestions-list

Conversation

@steipete

@steipete steipete commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Closes #876.

Summary

  • add gog docs suggestions list <docId> [--tab title-or-id]
  • request SUGGESTIONS_INLINE explicitly and emit pending text insertions/deletions with suggestion ID, segment, UTF-16 range, and text
  • combine contiguous runs, preserve nested suggestion IDs, and traverse tables, table-of-contents content, headers, footers, and footnotes
  • add stable JSON/TSV output, generated command references and service-skill metadata, user docs, changelog, and focused regressions

Style-only suggestions and accept/reject mutation remain intentionally out of scope. The Docs API response does not expose suggestion authors.

Proof

  • exact head d0e30f7e: make ci
  • exact head d0e30f7e: go test -race ./internal/cmd -run 'Test(DocsSuggestions|EnumerateDocsSuggestions)' -count=1
  • exact head d0e30f7e: generated docs/skills checks; 704 command pages and clean worktree
  • exact head d0e30f7e: structured branch autoreview clean (0.86); no accepted/actionable findings
  • built candidate: v0.31.1-10-gd0e30f7e; SHA-256 fd8d7de82059b51232eff664381408040f1a1c2db69b0c404b1a02d858635e5d
  • pre-rebase implementation head: built-binary authenticated smoke against a disposable Google Doc using the available dedicated test account; default and explicit-tab inline reads passed; temporary files trashed
  • pre-rebase implementation head: source-blind behavior validation passed help, invalid ID, empty JSON/plain read, tab selection, and unknown-tab failure
  • focused fixtures cover pending insertions/deletions, nested IDs, contiguous grouping, tables, non-body segments, exact request query parameters, tab projection, JSON, and TSV escaping

The rebase onto current main resolved only the changelog entry and refreshed the generated command-page count from 702 to 704; implementation code is unchanged from the live/behavior-tested head.

Live-proof gap

Google's Docs API can read but cannot create a pending suggestion. The existing-profile Chrome extension was unavailable and the approved DevTools bridge returned no real tabs, so an authenticated document containing a real pending insertion/deletion could not be created safely. The exact pending-change rendering path is therefore fixture-proven but awaits either a live fixture or an explicit item-specific maintainer waiver before merge.

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 1, 2026, 6:13 AM ET / 10:13 UTC.

Summary
The PR adds a read-only gog docs suggestions list command that requests inline Google Docs suggestions and emits pending insertion/deletion IDs, ranges, segments, and text with generated docs, skill metadata, changelog text, and tests.

Reproducibility: not applicable. as a bug reproduction: this PR adds a new read-only command. The source-level failure path is high-confidence because the added walker ignores structural suggestion ID fields that exist in the pinned Docs API model.

Review metrics: 2 noteworthy metrics.

  • Changed files: 11 files changed. The feature spans implementation, focused tests, generated command docs, user docs, changelog text, and skill metadata.
  • New focused tests: 205 test lines added. Fixture coverage is the main confidence source because the PR body calls out a live pending-suggestion proof gap.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #876
Summary: This PR is the candidate implementation for the open feature request to expose pending Google Docs suggestions from the CLI.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🌊 off-meta tidepool
Patch quality: 🦐 gold shrimp
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Handle structural suggestion IDs or document the narrower text-run-only scope before merge.
  • Have a maintainer decide whether fixture proof is enough without a live pending-suggestion document.

Risk before merge

  • [P1] The new command can silently omit pending suggestions attached to table, row, cell, or table-of-contents structures, so users may get an incomplete list for valid Docs API responses.
  • [P1] The PR body documents that an authenticated Google Doc containing real pending insertion/deletion suggestions was not available for after-fix proof; maintainers need to decide whether fixture coverage is enough.

Maintainer options:

  1. Handle structural suggestion IDs before merge (recommended)
    Update the walker and tests so table, row, cell, and table-of-contents insertion/deletion IDs are not silently dropped.
  2. Accept the first narrow text-run scope
    Maintainers may intentionally land only text-run suggestions if the docs and PR body clearly state that structural suggestion IDs are excluded for now.
  3. Pause for live fixture proof
    Hold the PR until a real document with pending suggestions can validate the exact live API shapes this command needs to summarize.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Fix `gog docs suggestions list` so it accounts for Google Docs structural suggestion IDs on tables, table rows, table cells, and table-of-contents nodes; add focused regression coverage for those shapes; keep the command read-only and avoid changing unrelated docs or generated files beyond what the command schema requires.

Next step before merge

  • [P2] A narrow automated repair can carry or emit structural suggestion IDs and add focused fixtures, while maintainers still own the feature contract and live-proof waiver.

Security
Cleared: The diff does not add dependency, workflow, credential, package-resolution, network-execution, or secret-handling changes.

Review findings

  • [P2] Carry structural suggestion IDs — internal/cmd/docs_suggestions.go:162-170
Review details

Best possible solution:

Carry structural suggestion IDs through the walker or emit structural-range items, then land the read-only suggestions surface after maintainer approval of the public command contract and proof bar.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction: this PR adds a new read-only command. The source-level failure path is high-confidence because the added walker ignores structural suggestion ID fields that exist in the pinned Docs API model.

Is this the best way to solve the issue?

No, not quite as submitted. The dedicated command is a maintainable shape, but the walker should account for structural suggestion IDs or explicitly narrow and document its supported scope before merge.

Full review comments:

  • [P2] Carry structural suggestion IDs — internal/cmd/docs_suggestions.go:162-170
    The Docs API exposes suggested insertion/deletion IDs on Table, TableRow, TableCell, and TableOfContents nodes as well as text runs. This loop only descends into their child content and later reads TextRun IDs, so a suggested table/row/cell or table-of-contents insertion can be silently omitted from gog docs suggestions list unless Google duplicates the ID onto every nested run.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d405310c351e.

Label changes

Label changes:

  • add merge-risk: 🚨 other: Green CI does not prove real Docs API structural suggestion shapes, and merging as-is could ship incomplete suggestion output.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR, though the PR body still documents a live pending-suggestion fixture gap for maintainers.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.

Label justifications:

  • P2: This is a bounded Docs CLI feature with a concrete linked request and limited blast radius.
  • merge-risk: 🚨 other: Green CI does not prove real Docs API structural suggestion shapes, and merging as-is could ship incomplete suggestion output.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR, though the PR body still documents a live pending-suggestion fixture gap for maintainers.
Evidence reviewed

Acceptance criteria:

  • [P1] go test ./internal/cmd -run 'Test(DocsSuggestions|EnumerateDocsSuggestions)' -count=1.
  • [P1] go test ./internal/cmd -count=1.

What I checked:

  • AGENTS policy read: Read AGENTS.md fully and applied its PR review rule to inspect with gh pr view/diff without switching branches or editing files. (AGENTS.md:1, d405310c351e)
  • PR metadata and protected author signal: GitHub reports this PR is open, cleanly mergeable, authored by steipete, and the provided item context marks the author association as COLLABORATOR, so cleanup close is not allowed even if the branch is stale or debatable. (d0e30f7ef2bc)
  • PR implementation walker: The added recursive walker descends into table cells and table-of-contents content but only appends IDs from paragraph text runs, so structural suggestion IDs are not emitted. (internal/cmd/docs_suggestions.go:162, d0e30f7ef2bc)
  • Pinned Docs API model has structural IDs: The pinned google.golang.org/api v0.285.0 model exposes SuggestedInsertionIds and SuggestedDeletionIds on Table, TableRow, TableCell, and TableOfContents, not only on text runs. (go.mod:20, d405310c351e)
  • Current main lacks the new list surface: Current main has DocsRawCmd and tab projection helpers, but DocsCmd does not register a suggestions subcommand and raw reads do not set SuggestionsViewMode by default. (internal/cmd/docs.go:18, d405310c351e)
  • Related request is canonical: The linked open issue requests the same read-only Docs suggestions surface and the PR body uses closing syntax for that issue.

Likely related people:

  • steipete: Local blame and git history tie the current Docs raw command, tab projection, tab helper, and enumerator helper lineage to recent Docs work by this person. (role: recent Docs raw/read area contributor; confidence: high; commits: 608aa46e7b9c, 0a7e6425384b, e51f6be9dc38; files: internal/cmd/docs.go, internal/cmd/docs_raw_test.go, internal/cmd/docs_enumerators.go)
  • kiranmagic7: Recent history for internal/cmd/docs_read.go includes the ambiguous tab read fix, adjacent to the tab-selection behavior reused by this PR. (role: recent adjacent Docs read contributor; confidence: medium; commits: 9cf00cbc00a9; files: internal/cmd/docs_read.go)
  • Alex Hillman: Earlier history introduced Docs tab support and read/edit command structure that tab-aware suggestions need to preserve. (role: Docs tabs/read feature introducer; confidence: medium; commits: 2f5131995552; files: internal/cmd/docs.go, internal/cmd/docs_commands_test.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jul 1, 2026
@steipete steipete force-pushed the codex/docs-suggestions-list branch from eed3903 to d0e30f7 Compare July 1, 2026 09:12

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0e30f7ef2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +162 to +166
if element.Table != nil {
for _, row := range element.Table.TableRows {
for _, cell := range row.TableCells {
walk(cell.Content)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle table-level suggestion IDs

When the pending edit inserts or deletes an entire table, row, or cell, the Docs model can attach suggestedInsertionIds/suggestedDeletionIds to the Table, TableRow, or TableCell rather than only to each nested TextRun. This branch only descends into cell.Content and later looks at element.TextRun IDs, so those structural suggestions are silently omitted unless the API also duplicates the ID onto every run; users can get an empty or partial list for suggested table edits. Please carry active table/row/cell suggestion IDs into the recursive walk or emit items for the structural range.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(docs): read-only surface for suggested edits (tracked changes) — docs suggestions list / --suggestions

1 participant