Skip to content

Support Docs smart chips in cat output#909

Closed
TurboTheTurtle wants to merge 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/docs-cat-smart-chips
Closed

Support Docs smart chips in cat output#909
TurboTheTurtle wants to merge 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/docs-cat-smart-chips

Conversation

@TurboTheTurtle

Copy link
Copy Markdown
Contributor

Closes #907.

Summary

  • Keeps existing plain-text output unchanged by default.
  • Adds --chips for inline person/date/rich-link rendering.
  • Adds additive renderedText and chips fields to JSON output when smart chips are present.

Evidence

  • make ci passes.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 12:24 AM ET / 04:24 UTC.

Summary
The branch adds a --chips option for gog docs cat, emits additive renderedText and chips JSON data for smart chips, adds unit coverage, updates command docs, and includes a tiny unrelated Forms constant cleanup.

Reproducibility: yes. at source level. Current main's extractor skips non-TextRun paragraph elements, so the linked person/date/richLink chip repro would be omitted from plain and JSON Docs cat output; I did not run a live Google Docs reproduction in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Changed files: 4 files affected. The PR touches the Docs read path, command docs, tests, and a small unrelated Forms cleanup reviewers should notice.
  • New output surface: 1 flag, 2 additive JSON fields. The changed behavior affects a script-facing command contract even though existing default text is preserved.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #907
Summary: This PR explicitly proposes a fix for the canonical Docs smart-chip readback issue.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted real behavior proof for gog docs cat --chips and gog docs cat --json on a Google Doc containing person, date, and rich-link chips.
  • Confirm maintainer acceptance of the opt-in --chips plus additive JSON output contract.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports only make ci; the contributor should add redacted terminal output, logs, or a recording showing gog docs cat --chips and gog docs cat --json against a real smart-chip document, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] External real-behavior proof is missing; unit tests and make ci do not show the feature working against a live Google Docs document containing smart chips.
  • [P1] The PR adds a new CLI flag and additive JSON fields on a script-facing command, so maintainers should explicitly accept that output contract before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land a chip-aware Docs read extractor that preserves existing default text, exposes explicit rendered text and structured chip metadata, and is proven against a real smart-chip document before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Keep this PR open for contributor proof and maintainer acceptance of the output contract; there is no narrow automated repair task identified.

Maintainer decision needed

  • Question: Should gog docs cat use this opt-in --chips flag plus additive JSON fields as the accepted smart-chip readback contract?
  • Rationale: The linked issue was previously routed as needing an output-contract choice, and this PR adds a new flag plus JSON fields on a script-facing command without visible maintainer confirmation yet.
  • Likely owner: steipete — Most connected to the current Docs read extractor and existing smart-chip insertion surface in git history.
  • Options:
    • Accept opt-in/additive contract (recommended): Proceed with the compatibility-preserving --chips flag and additive JSON fields after the contributor adds live smart-chip proof.
    • Prefer default rendering: Ask for chips to render in the existing text fields by default, accepting the script-visible behavior change as intentional.
    • Defer product surface: Pause or close this PR until maintainers choose a different Docs readback contract.

Security
Cleared: The diff changes CLI rendering, tests, and docs only; it does not add dependencies, workflows, credential handling, or code-execution paths.

Review details

Best possible solution:

Land a chip-aware Docs read extractor that preserves existing default text, exposes explicit rendered text and structured chip metadata, and is proven against a real smart-chip document before merge.

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

Yes at source level. Current main's extractor skips non-TextRun paragraph elements, so the linked person/date/richLink chip repro would be omitted from plain and JSON Docs cat output; I did not run a live Google Docs reproduction in this read-only review.

Is this the best way to solve the issue?

Likely yes, pending maintainer direction. The PR preserves existing default text while adding opt-in rendering and structured JSON chip data, but the new output contract and live behavior proof still need confirmation.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority Docs read/output bug fix with limited blast radius and a clear linked issue.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports only make ci; the contributor should add redacted terminal output, logs, or a recording showing gog docs cat --chips and gog docs cat --json against a real smart-chip document, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Repository policy read: Read the full target AGENTS.md and applied its PR-review guidance plus parseable-output guidance to this review. (AGENTS.md:1, a1977274de23)
  • Current main skips chip elements: Current appendDocsElementText continues past paragraph elements without TextRun, which excludes person, date, and richLink chip elements from normal Docs cat extraction. (internal/cmd/docs_read.go:311, a1977274de23)
  • PR implements chip rendering path: The PR diff adds Chips, rendered-text helpers, smart-chip rendering for person/date/richLink elements, and JSON fields populated from that rendered result. (internal/cmd/docs_read.go:23, 00f47e4c2717)
  • PR covers expected cases in tests: The PR adds an httptest Docs response with person, date, and richLink paragraph elements, then checks default text remains unchanged while --chips and JSON expose chip content. (internal/cmd/docs_commands_test.go:682, 00f47e4c2717)
  • Linked issue is the canonical bug: The linked issue describes the same missing readback behavior and GitHub reports this PR as its closing pull request reference.
  • Real behavior proof is still missing: The PR body lists only make ci as evidence, and the visible comments do not include terminal output, logs, screenshots, recordings, or artifacts from a live Google Docs smart-chip document. (00f47e4c2717)

Likely related people:

  • steipete: GitHub history shows repeated Docs read refactors and the merged smart-chip insertion surface, including document shaping helpers and smart-chip insertion support adjacent to this readback behavior. (role: feature owner and recent area contributor; confidence: high; commits: 96529accf58f, 3bed2b06e788, 9f2fae2913c4; files: internal/cmd/docs_read.go, internal/cmd/docs_smart_chips.go, internal/cmd/docs_commands_test.go)
  • kiranmagic7: Authored the recent ambiguous-tab validation change touching docs cat implementation and tests, adjacent to the paths this PR changes. (role: recent Docs cat contributor; confidence: medium; commits: 9cf00cbc00a9; files: internal/cmd/docs_read.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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-09T03:48:53.529Z sha abc88da :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 9, 2026
@TurboTheTurtle TurboTheTurtle force-pushed the fix/docs-cat-smart-chips branch from abc88da to 00f47e4 Compare July 9, 2026 04:21
steipete added a commit that referenced this pull request Jul 11, 2026
Closes #907

Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
@steipete

Copy link
Copy Markdown
Collaborator

Landed on main as f249739. Thank you, @TurboTheTurtle!

Maintainer follow-up strengthened numbered/tabbed output coverage, atomic byte-limit behavior, fallback handling, Markdown delimiter escaping, and MCP untrusted-content wrapping. Proof: focused smart-chip tests, full make ci, clean autoreview, and a disposable live Google Docs fixture containing native person, date, and Drive-file chips; default output remained unchanged, opt-in text/JSON rendered all three, and all fixtures were trashed.

@steipete steipete closed this Jul 11, 2026
@TurboTheTurtle TurboTheTurtle deleted the fix/docs-cat-smart-chips branch July 11, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs cat drops person smart chips from all output modes (plain, --plain, --json); readable only via --raw

2 participants