Module implementation history

Per-PR history behind the rules in the root CLAUDE.md: review rounds, reversed approaches, live-QA findings, and the incident behind each rule. It was moved here verbatim from CLAUDE.md in #849 so that file could hold only the lasting rules and invariants, which cost every session ~24k tokens at its old size.

Each section below is the full original text of one CLAUDE.md entry, under the name CLAUDE.md uses for it. Entries short enough to have no history stayed in CLAUDE.md unchanged and aren't repeated here. Read the section for a module before changing that module's behavior: the invariants in CLAUDE.md are the summary, and the reasoning and edge cases are here.

Issue and PR numbers are as they were at the move (2026-09-29). Later history belongs in the module's own design or decision doc, not appended here.

tools/drive/files.py

tools/drive/files.py — file/folder operations (create_spreadsheet, import_csv_to_sheet, list_spreadsheets, list_files, search_files, create_folder, create_shortcut, copy_file, move_file, rename_file, star_file, unstar_file, delete_file, restore_file, empty_trash, list_shared_with_me, list_recent_files, get_storage_quota, etc.). star_file/unstar_file (#139) set starred via files().update — get_file_metadata, list_files, search_files, and search_spreadsheets all surface the starred field back (#388), and star_file/unstar_file mark the file's parent folder(s) dirty in drive_folder_cache the same way rename_file does, so a subsequent list_files reflects the new state without a manual refresh_cache (see TC-D202/203/260 in docs/qa/tests/drive_files.md). Both list_files and get_file_metadata now surface Drive's md5Checksum field as md5_checksum (#274) — None for Google Workspace files (Docs/Sheets/Slides have no fixed byte content, so the Drive API omits the field for them), a real hash for binary files. Exists so callers can detect real content drift instead of inferring it from modifiedTime, which sync/upload paths don't always keep faithful to actual content (see transfer.py's sync_folder use_checksum param below, the primary consumer of this).

tools/drive/transfer.py

tools/drive/transfer.py — upload/download/sync/export/revisions; _upload_local_file (the core behind the upload_local_file tool) is a module-level helper so docs/images.py's insert_local_images can call it directly, imported cross-package the same way docs/content.py imports _SA_QUOTA_ERROR from tools/drive/__init__.py. upload_local_file's convert param (#188) requests Drive's native import conversion — CSV/XLSX→Sheets, DOCX/MD/HTML→Docs, PPTX→Slides — by uploading with the source format's mimeType while setting the destination's mimeType metadata to the target Google Workspace type (_CONVERT_MIME maps extension → (source mime, target mime)). This is a different mechanism from docs/content.py's create_doc_from_file, which parses the file locally and rebuilds it via Docs API requests instead of Drive's importer — don't conflate the two when adding new conversion paths. sync_folder's convert_markdown param (#211) applies the same native-conversion mechanism to local .md files during sync uploads. Because the converted Doc keeps its .md name in Drive, _sync_level's drive_map-building loop matches it back to the local .md file directly (bypassing the export_format-suffix scheme normal Workspace files need) so resyncs settle into "in sync" instead of re-uploading a duplicate every run — this also reopens matching for a drive-only converted Doc with no local counterpart and no export_format (previously unreachable, since such files were always excluded from drive_map) — the plan-building loop classifies that case as a conflict (a convert_markdown Doc has no reverse conversion) before it ever reaches the download branch, so the download branch's own _EXPORT_MIME[None] guard is defensive-only dead code in the current implementation, not a reachable path (confirmed live: TC-D221). There is no reverse conversion (Doc → markdown). A converted Doc is recognized on every later sync via a Drive properties marker (_CONVERT_MARKDOWN_SOURCE_PROP) stamped at creation time by both _sync_level's own upload path and _upload_local_file's convert=True path (#414 rounds 2-3) — recognition is deliberately independent of whether the current call passes convert_markdown/convert, since the Doc's own persisted property is what matters, not this call's flag. Both paths stamp through the shared _convert_properties helper, which keeps every value within Drive's 124-byte per-property cap: _CONVERT_MARKDOWN_SOURCE_PROP holds the raw name only when it fits, and the generic _CONVERT_SOURCE_PROP marker (#769) falls back to a sha256: digest. _sync_level used to stamp the raw name directly, so a .md name over 95 bytes failed create() with 403 propertyLengthLimitExceeded after Drive had already made the Doc, leaving an orphan on every run (#805). Recognition goes through _converted_md_source_name, which accepts either key. The legacy markdown key is still honored, so Docs converted before #805 keep matching. The generic marker counts only for a .md source, and a digest is resolved against the Doc's own display name, which keeps its .md suffix. Because of that, a digest-marked Doc (a name over 103 UTF-8 bytes, .md included) stops matching if it's renamed in Drive; see docs/known-limitations.md. Three further edge cases from that round-3 review shipped as issue #422: the drive_map-building loop now detects (rather than silently resolving) a name collision between a plain file and a converted Doc, reporting it under failed instead of letting whichever is listed last win the slot and make the other invisible to the sync; _upload_local_file now stamps modifiedTime from the local file's mtime on a converted Doc (via create() then a metadata-only update(), mirroring _sync_level's own create() branch, since Drive's native import-conversion overwrites the create()-time value once conversion finishes) so a subsequent sync_folder lands in skipped rather than conflicts; and the drive-only branch now checks direction before _is_converted_md, so direction='upload' reports a plain skip like any other drive-only file instead of always reporting conflict. PR #433's own review round found the collision guard needed to go further: it originally only fired when the two colliding entries' _is_converted_md differed (a plain file vs. a converted Doc, #422's own reported shape), silently overwriting a same-type collision (two plain files, or two converted Docs, sharing a name) the exact same way; it now fires on any duplicate name regardless of type. The collision case is also now routed through the normal plan/action machinery (a new "collision" action, mapped to conflict during dry_run and to a real failed entry — kind: "collision_fail" — once execution is actually attempted) rather than writing to failed directly from the drive_map-building loop: that direct write both leaked into failed during dry_run (breaking the "dry_run never reports failures, only previews" invariant the pre-existing folder-collision failure path already upholds) and silently dropped a local file sharing the colliding name from every result list, since the plan loop's bare continue skipped it with zero acknowledgment. Separately, _upload_local_file's create()+update() pair now shares one broad except Exception returning {"error": ...} (matching _sync_level._run_one's own create+update pattern) instead of only catching quota HttpErrors and letting everything else — including a failure in the new metadata-only restamp update(), after create() already succeeded — propagate uncaught to upload_local_file, which has no try/except of its own. Issue #420 found that same broad except still lost information on the restamp-failure path specifically: a bare {"error": ...} (or, for _sync_level._run_one's own mirrored pair, a bare upload_fail) gave no way to find the Doc that create() had already genuinely made in Drive before the restamp failed — an untracked orphan. Fixed in both places by giving the restamp update() its own dedicated try/except that reports the created file's own ID alongside the error — _upload_local_file as {"error": ..., "fileId": ...}, _run_one as {"kind": "upload_fail", "name": ..., "error": ..., "fileId": ...} — while a create() failure still reports no fileId, correctly, since nothing was created. The two functions structure this differently: _run_one's restamp try/except sits nested inside its one broad outer try (which also covers create() and the if name in drive_map update branch above it), while _upload_local_file's is a separate, sequential try/except reached only after the create() try/except has already returned successfully — not nested inside it (PR #645 QA round 1 caught this file's prose originally describing both as nested). PR #650 factored the two restamp-failure branches' shared shape — the created Drive file {id!r} but failed to restamp its modifiedTime message plus storageQuotaExceeded→_SA_QUOTA_ERROR rendering — into a module-level _restamp_failure_result(file_id, exc) returning {"error": ..., "fileId": ...}; _run_one spreads that and layers its own kind/name keys on top. Before this only the create() except recognized a quota error — the restamp except caught bare Exception and would have leaked a raw str(e) for a quota failure there, unlike every other quota-error site in the file. sync_folder's and upload_local_folder's own failed-list aggregation steps (which previously narrowed each per-item result down to just {name, error}) were updated to carry fileId through when present, or the fix would have been invisible by the time a caller saw the result — and, since a fileId-carrying failure means create() genuinely changed the folder's contents, all three call sites (_sync_level, the upload_local_file tool, upload_local_folder) also had to treat that case as cache-dirtying, the same as a real success: none of them originally did (same QA round), since a fileId-carrying result still fell into each site's own success-only cache-invalidation gate's "false" branch. sync_folder's use_checksum param (#274) adds a content check ahead of _sync_level's mtime comparison for names present on both sides: when the local file's md5 (_local_md5, streamed in 1 MiB chunks) matches Drive's own md5Checksum (now fetched by _list_drive_children), the pair is treated as in sync regardless of modifiedTime drift — this is what actually fixes upload_local_file not stamping modifiedTime (unlike _sync_level's own upload path) causing a spurious re-download of identical content on the next sync, not just a same-mtime coincidence. Only applies where Drive returns a real md5Checksum — Workspace files and convert_markdown Docs have none and always fall back to mtime-only comparison, flag or no flag. A checksum mismatch doesn't force any decision by itself; it just falls through to the same mtime-based upload/download/conflict logic used when use_checksum=False, since content differing doesn't by itself say which side is newer. PR #472's own review round found four issues in this first version: the checksum comparison ran unconditionally, before the (cheap) mtime diff, including for pairs already within _SYNC_MTIME_TOLERANCE and even during dry_run=True — fixed by moving the mtime diff first and gating the hash read on use_checksum and not dry_run and abs(diff) > _SYNC_MTIME_TOLERANCE, so an already-in-sync pair or a dry_run preview never pays for a read it can't change the outcome of (#716 later dropped the abs(diff) part of that gate; see below); _local_md5 had no try/except unlike every other per-item operation in this module, so a file that became unreadable (deleted, permission-denied) between the directory scan and the hash read raised uncaught out of the whole sync_folder call instead of degrading to one failed entry — fixed via a new plan action (now "local_read_fail", which since PR #841 also covers the plan's own stat()) mirroring the existing "collision" pattern; and _upload_local_file's plain (non-convert) branch was identified as the actual root cause worth fixing directly — it never stamped modifiedTime the way the convert_mime branch three lines above already did (and #422 already established for that branch), so a file uploaded through it always read as "Drive newer" on the very next sync_folder regardless of content — now stamped unconditionally in the create() body for every upload (no follow-up update() needed for the plain case, since Drive only overwrites modifiedTime when import-conversion is involved). use_checksum remains valuable beyond that root-cause fix for causes of drift the mtime stamp doesn't cover (e.g. a local overwrite that happens to preserve mtime). One review finding was accepted as a known tradeoff rather than fixed: a checksum mismatch that resolves to 'upload' reads the local file twice (once to hash, once via MediaFileUpload for the actual transfer) — avoiding it would mean buffering the whole file in memory to share across both reads, a worse tradeoff for large files than one extra (likely page-cache-warm) disk read. Issue #659 covered the inverse of use_checksum's target failure mode — false sameness, where the two mtimes agree but the content has diverged (classic cause: a rename-in-place, since mv preserves mtime), which short-circuited to "in sync" before the hash block (then guarded on abs(diff) > _SYNC_MTIME_TOLERANCE) could run, so use_checksum=True couldn't reach it by construction (until #716, below). Fixed with a near-free discriminator in _sync_level's both-sides branch: size is now also fetched by _list_drive_children, and a within-tolerance mtime pair whose local st_size disagrees with Drive's reported size is not skipped — it is reported as a conflict for every direction, never an auto-transfer (PR #712 QA round 1): the mtimes agree so recency is unknown, and the diff-based branches already report conflict rather than overwrite a target they can tell is newer, so this branch — which knows less — must be at least as cautious. The byte-size check is one stat() and no read, so unlike the hash block it runs during dry_run too. Non-Workspace files only (size, like md5Checksum, is absent for Workspace/convert_markdown files). A same-size content edit that also preserves mtime still reads as "in sync" under the default use_checksum=False. Issue #716 closed that gap for an explicit use_checksum=True: it now hashes every both-sides pair with a Drive md5Checksum, within tolerance or not (a size mismatch is still settled from the stat() alone, first). A within-tolerance hash mismatch is a conflict for every direction, the same as the size mismatch; both come from _diverged_same_mtime_step. The hashes run after the plan loop, concurrently, capped at _SYNC_HASH_CONCURRENCY, each filling in a placeholder step that holds the mtime-only fallback from _mtime_step. dry_run still never hashes, and an affected step's reason ends in (checksum not verified in dry_run), since the real run can decide differently. upload_local_folder (#411) used to have its own independent inline upload loop with no convert param, unreachable from _upload_local_file's conversion mechanism — bulk-importing a folder of CSV/DOCX/PPTX into native Workspace formats required falling back to per-file upload_local_file calls. Fixed by routing each candidate through _upload_local_file itself, gaining the same convert param and its semantics (an unsupported extension lands in failed rather than uploading as-is, matching upload_local_file's own error-not-silent-fallback behavior). The existing bulk skip_if_exists pre-check (one list() call for the whole folder, not one per file — TC-D100) is preserved rather than replaced by _upload_local_file's own per-call check: the folder loop now fetches each existing file's mimeType alongside its name in that same single call, decides skip-worthiness itself (name-only match when convert=False; name and target-mimeType match when convert=True, mirroring _upload_local_file's own convert-aware duplicate check), and then calls _upload_local_file with skip_if_exists=False for whatever survives — so existence is never checked twice. sync_folder(dry_run=True)'s response used to duplicate every skip/conflict name into both a flat list (skipped/conflicts) and a full {name, action, reason} entry in actions, deterministically tripping the response-size cap on a moderately-sized, mostly-in-sync folder even though actions alone (name + action-type + reason for every item, including upload/download entries the flat lists never populated during dry_run in the first place) was already a complete, non-redundant picture (issue #512). Fixed by leaving uploaded/downloaded/skipped/conflicts/failed empty during dry_run rather than trimming or toggling actions itself. Separately, sync_folder previously had no response-size-cap bypass at all (local_path_available=False) unlike every other capped tool except export_file/list_file_activity (both deliberately, per the decision doc below) — its own local_path param already means the sync destination, so it can't double as a dump target the way get_sheet_data's local_path does. Fixed by adding a separate result_local_path param following the same unconditional-if-passed write_capped_result_to_disk pattern as every other capped tool with a bypass; enforce_response_size_cap gained a local_path_param override so its error message names the right parameter instead of hardcoding local_path. PR #518's own QA review round live-caught a gap in that first version: result_local_path had no guard against pointing at or inside local_path — since local_path is scanned as sync input on every call (unlike every other capped tool's local_path/result_local_path, which are pure output destinations), a manifest written there showed up as a new local-only file on the very next sync and would have been uploaded to Drive on a real run. Fixed by rejecting the call up front (ValueError, before any Drive API call) when Path(result_local_path).resolve() equals or is a descendant of the resolved local_path. upload_file's convert_to_doc param implements the identical native-import-conversion trick as _upload_local_file's convert/_CONVERT_MIME above, independently — raw text/markdown/html content instead of a local file, always targeting a Doc since there's no file extension to dispatch Sheets/Slides on — with no cross-reference between the two (issue #412). Fixed by sharing _GOOGLE_DOC_MIME as the single source of truth for the target mimeType literal both mechanisms write, plus explicit docstring cross-references at both call sites; the two code paths themselves stayed separate rather than being merged, since their actual upload semantics (raw content vs. local file, single target vs. extension-dispatched) differ enough that unifying them would cost more complexity than it'd save. Issue #424 (folded #421's two extra findings in) was pure hardening on this same convert_markdown matching logic, no behavior change: the synthetic "_is_converted_md" key spread into every drive_map entry (previously read directly at 5 separate call sites — the pattern PR #433's own review round flagged as having already caused one missed site) was replaced with _is_converted_md_entry(f), a helper that recomputes the classification on demand from the resource's own mimeType/properties rather than caching it — drive_map is now a plain passthrough of Drive's file resource shape again, and every call site (the build loop, both plan conflict branches, the upload-execution reimport check, the download-execution guard) shares one place to agree with when this classification's rules change. The three independently-hand-written "no reverse conversion" messages (drive-only plan conflict, drive-newer plan conflict, download-execution guard) now interpolate a shared _NO_REVERSE_CONVERSION_CLAUSE constant instead. The dead assert convert_source is not None fell out naturally once the build loop's local_name derivation was rewritten against the new helper. The upload-execution branch's _CONVERT_MIME[".md"] unpack moved inside the convert_this check so it's only looked up for an actual .md conversion, not unconditionally for every upload. The doubled create()+update() latency on every new converted-.md upload (both here and in _upload_local_file's convert=True path) was reviewed and left as an accepted tradeoff, not fixed — Drive's import-conversion finishes asynchronously after create() already returns, so there's no single call that can request both the upload and the post-conversion modifiedTime restamp together, and polling for conversion completion before restamping would trade one predictable extra call for an unpredictable number of slower ones. Issue #814 took converted .md Docs off the modifiedTime comparison entirely: Drive updates a Doc's modifiedTime from the Docs backend asynchronously, minutes behind, which both overwrote the restamp (an unedited Doc sat in conflicts) and hid real Drive edits for minutes. Every converted-.md upload now stamps _CONVERTED_MD_SOURCE_MTIME_PROP (the local mtime), a re-upload also stamps _CONVERTED_MD_BASELINE_REV_PROP (the Doc's latest revision, read just before the update()) and clears _CONVERTED_MD_IMPORT_REV_PROP, and the first sync afterward records the import's revision ID there. _sync_level then judges the local side against the stamped mtime and the Drive side from revisions.list (_converted_md_drive_state/_converted_md_step, fetched concurrently after the plan loop and capped at _SYNC_REVISION_CONCURRENCY, during dry_run too). A Drive edit is always a conflict, including when the local file changed too (previously an upload over the Drive edit). A revision-read failure is a "drive_read_fail" plan action. A Doc without the stamp (converted before #814) keeps the modifiedTime path until its next re-upload. The restamp stays, now cosmetic for these Docs, and the import-revision write re-sends the stamped modifiedTime, since a property-only update bumps it to "now" (seen live). PR #854 QA round 1 tightened it: the source-mtime stamp keeps microsecond precision and is compared exactly (_CONVERTED_MD_STAMP_TOLERANCE), since the 5s Drive clock-skew tolerance hid a local save made within 5s of the upload. The import-revision write always re-sends a modifiedTime, since a property write bumps it to "now": the stamped source mtime for an unedited Doc, and Drive's own listed value for a Drive-edited one (re-sending the source mtime there erased the edit's timestamp). It also marks the folder cache dirty. An empty revision history is a drive_read_fail, not an edit. A local change under direction='download' is a conflict even while the import is pending. The upload path's fresh revision read now aborts the upload as a conflict if the latest revision moved since planning. Every upload also stamps _CONVERTED_MD_UPLOADED_AT_PROP, so a pruned baseline falls back to finding the import by timestamp, and a candidate import more than _CONVERTED_MD_IMPORT_WINDOW after the upload is read as the import merged into a later edit (changed). Legacy Docs already stuck in the drift conflict are unstuck by touching the local file (docs/known-limitations.md). Design: docs/decisions/decision-converted-doc-change-detection.md; other conversion types are #818.

tools/docs/content.py

docs/content.py — helpers (_md_to_html, _to_doc_requests, _html_to_doc_requests) and tools (create_doc, get_doc_content, write_doc_content, create_doc_from_file, update_doc_from_file, get_doc_structure, find_in_doc). insert_doc_text/delete_doc_range/insert_page_break/insert_softbreak_paragraph moved to docs/editing.py, and insert_inline_image/insert_local_images moved into docs/images.py (#372, following #371's extraction of named_ranges.py) — _collect_doc_paragraphs moved alongside them into docs/indices.py, next to its own sibling low-level helper utf16_len, and the dead _html_to_text helper (no callers anywhere in the codebase, confirmed via a repo-wide grep before deleting it) was dropped rather than moved. update_doc_from_file (#341) combines create_doc_from_file's server-side file reading (extension inference to markdown/html, or an explicit content_format override) with write_doc_content's in-place clear+replace — the clear-then-insert mechanism (including the same-batch insertText style-inheritance quirk described below) was extracted out of write_doc_content into a shared _replace_doc_content helper so both tools have exactly one implementation of it, the same way _apply_doc_content is already shared by create_doc/create_doc_from_file/write_doc_content. find_in_doc searches paragraph text (including table cells) and returns start_index/end_index offsets in UTF-16 code units usable directly with style_doc_range — _collect_doc_paragraphs is a generator (so a caller can stop early once max_results is hit) that walks the Docs API body, trusting each ParagraphElement's own startIndex when present and otherwise carrying the running offset forward rather than dropping that element, since the API doesn't always populate it (seen on a document's first element). get_doc_structure now also surfaces each paragraph's headingId (paragraphStyle.headingId) — confirmed live present on the default single-tab documents.get() response for both TITLE/SUBTITLE and HEADING_1..6 paragraphs, no includeTabsContent needed. create_doc/create_doc_from_file/write_doc_content now run an automatic second pass (_resolve_heading_anchors, gated by the cheap _has_pending_anchor_links check so content with no #-links skips the extra API round trip entirely) after any conversion that left a GitHub/GitLab-style #slug heading-anchor link behind (issue #409): it resolves the anchor against the doc's own real headings via anchors.resolve_heading_anchor and rewrites it to a working .../edit?tab=t.0#heading=<headingId> jump link (an ordinary URL is sufficient — no Link.headingId API field needed, confirmed live), or strips the link (keeping the text plain) when no heading matches with reasonable confidence — a link silently pointing at the wrong section is worse than a plain, unlinked mention. This was a deliberate product decision to resolve anchors automatically rather than only exposing headingId and leaving matching to the calling agent — see docs/design/heading-anchor-resolution.md for the full rationale. PR #453's own QA round (/code-review high + live testing against a fixture with no table and no anchor on the doc's first element) caught two correctness bugs the first version's hand-rolled body walk had: it only inspected top-level paragraph elements (a heading or anchor link living inside a table cell was invisible to both _has_pending_anchor_links and _resolve_heading_anchors), and it required a run's own pe.get("startIndex")/endIndex to both be non-None (silently dropping an anchor that is, or is part of, the document's first body element — the Docs API omits startIndex there, the same quirk _collect_doc_paragraphs already handles). Fixed by _walk_headings_and_anchor_runs, a dedicated recursive walker mirroring _collect_doc_paragraphs's own two correctness properties (table-cell recursion; carrying the running offset forward via character-by-character advance rather than trusting a run's own possibly-inconsistent endIndex) rather than reusing _collect_doc_paragraphs itself, since that generator's yielded shape (text + char-indices only) doesn't carry per-run link URLs or heading-paragraph style info and is used elsewhere (find_in_doc) in a way not worth risking a signature change for. _has_pending_anchor_links gained a second parameter (tables: list[Table], walked via _table_has_pending_anchor_link, recursing into nested tables) since fill_tables() builds and executes table-cell requests separately from content_requests, invisible to the original single-argument guard. create_doc/create_doc_from_file/write_doc_content now support inline images in markdown (![alt](src)) and HTML (<img>) conversion (issue #333) via a shared _apply_doc_content helper: src may be a local filesystem path (uploaded to Drive), a drive:<file_id> reference to an already-uploaded file, or a public http(s):// URL (used directly) — resolved by _resolve_image_source, sharing local-path/drive: sources anyone:reader the same way insert_local_images already does, then (per explicit product decision, and now also insert_local_images's own default — a behavior change from its original #332 shipped version) revoking that temporary share once the image is embedded, unless revoke_sharing=False. The Docs API rejects an entire batchUpdate atomically if any one insertInlineImage request in it can't be fetched (confirmed live via TC-DOC102's own deliberately-unreachable-URL case) — _apply_doc_content retries with the failing request stripped (its index is parsed directly out of Google's own error message) rather than losing every other request in the same call; see docs/design/image-conversion.md for the full design, including why Image is a zero-width AST node (not a marker) and the deliberate table-cell-image gap.

tools/docs/editing.py

docs/editing.py — tools (insert_doc_text, delete_doc_range, insert_page_break, insert_softbreak_paragraph), extracted out of content.py (#372) as content.py's text-range editing primitives, distinct from that file's remaining whole-document create/read/write concern. insert_softbreak_paragraph joins multiple lines with a literal "\v" (vertical tab) character in a single insertText call — confirmed live that the Docs API stores this as one paragraph element rather than splitting it, matching the Docs UI's own Shift+Enter soft-break convention — and sets namedStyleType explicitly over the inserted span rather than leaving it to whatever the insertion point would otherwise inherit.

tools/docs/images.py

docs/images.py — tools (insert_inline_image, insert_local_images) plus inline-image size validation/downscaling/error-clarification shared by every insertInlineImage call site (issue #400). The two tools moved here from content.py (#372) rather than into a differently-named module — issue #372 predates #400's creation of this file, and by the time it was picked up, images.py already existed here for the size-validation helpers, so merging avoided colliding with a newly-invented module name (an earlier version of this rationale claimed the two tools were already this module's "primary callers"; that doesn't hold up — content.py's own _resolve_image_source, which stays there, depends on the same shared helpers just as heavily). insert_local_images uploads local image files and swaps each into the doc at a plain-text marker in one call; confirmed live that insertInlineImage with a drive_file_id fails with "There was a problem retrieving the image" unless the file is first shared anyone/reader — the Docs backend fetches the image as an anonymous HTTP request, so being accessible to the authenticated user alone is not enough (this also applies to insert_inline_image's own drive_file_id path). Google Docs rejects any inline image over ~25 megapixels or ~50MB with a bare HttpError 400 "...The provided image is too large." naming neither the actual limit nor the image's own size (confirmed against Google's documented ceiling: https://developers.google.com/workspace/docs/api/how-tos/images, 25 megapixels / 50MB / PNG-JPEG-GIF only). check_dimensions/check_image_file pre-validate and return a clear {"error": ...} naming the limit and the image's actual size before any upload/share/embed call happens; downscale_image_bytes (Pillow — see docs/decisions/decision-pillow-image-dependency.md for why it was added as a dependency) resizes in place, preserving format, and declines (None) rather than corrupting an animated GIF (Image.resize only touches the current frame). check_image_file (then check_image_bytes, a bytes-in predecessor replaced in #560) originally only enforced the megapixel limit, not the byte-size one the module's own docstring already cited — a low-compressibility image (e.g. noise data) could be well under 25 megapixels but still over 50MB, passing pre-validation silently and only failing at the real batchUpdate call (issue #562, confirmed live via a real ~75MB-but-24-megapixel PNG). Fixed by adding check_file_size/too_large_bytes_message (the byte-size counterpart to check_dimensions/too_large_message) into it, and extending downscale_image_bytes with a bounded iterative shrink loop for the case where the megapixel-based scale factor alone is a no-op (the image is already within that limit) but the encoded result is still over the byte limit — unlike pixel count, encoded byte size isn't predictable from a single scale factor since it depends on compression, not just dimensions. The two Drive-metadata-only size checks (insert_inline_image's drive_file_id path here, _resolve_image_source's drive: path in content.py) were consolidated into one shared check_drive_image_metadata helper reading both imageMediaMetadata and Drive's own size field, replacing what had been the same check_dimensions-only pattern duplicated across both files. PR #580's own QA round 1 caught that rewrite_too_large_error — the fallback for uri-sourced images, which skip pre-validation entirely — still only named the 25-megapixel limit, even though a byte-size-caused rejection could now reach it too; since Google's raw error text doesn't say which limit was actually hit and this call site never fetched the bytes to check, the message was made limit-agnostic (names both ceilings) rather than guessing which one applies. insert_inline_image's drive_file_id path, insert_local_images' local_path entries, and _resolve_image_source's drive:/local-path branches (content.py) all gained an opt-in auto_downscale param (default False, fail-fast with the clear error) wired to this module — a drive_file_id/drive: source that gets downscaled creates a new " (resized)"-suffixed copy alongside the original via downscale_drive_file (the original file is never mutated), while a local-path source just uploads the resized bytes directly via upload_and_share_image instead of the original file, since there's no pre-existing Drive artifact to avoid colliding with. Issue #560 deduplicated those call sites: downscale_drive_file takes the source's own Drive metadata and owns the resized copy's parent-folder fallback, and prepare_local_image owns the local-path check→downscale→upload sequence, so insert_inline_image, insert_local_images, and _resolve_image_source each only record the outcome. check_image_file reads just stat() and the image header, never the full file or its pixels, so an under-limit local image is read from disk once, by the upload itself. For PNG/JPEG/GIF it constructs Pillow's format plugin directly instead of going through Image.open, whose decompression-bomb guard would otherwise make a header claiming more than ~179 megapixels unreadable and silently skip pre-validation (PR #801 QA round 1); the file itself is opened outside the best-effort guard, so an unreadable file still reports a read error rather than passing as "not an image." downscale_image_bytes runs via asyncio.to_thread, since a full decode + resize + re-encode is CPU-heavy. Deliberately out of scope: a bare http(s):// image source (either insert_inline_image's own uri param or a markdown/HTML ![alt](http://...)/<img src="http://...">) — pre-validating or auto-downscaling it would mean fetching arbitrary external content just to check it, and re-hosting a resized copy somewhere Google can fetch it from is a bigger behavior change than this fix warrants. That source kind instead gets rewrite_too_large_error's fallback: Google's own "too large" HttpError message is caught and rewritten with the same limit/size explanation once the embed actually fails, rather than staying opaque.

tools/docs/anchors.py

docs/anchors.py — resolve_heading_anchor, the pure slug-matching logic behind the anchor-resolution pass above. Tries GitHub's non-collapsing hyphen convention (hand-rolled — no known Python package reproduces it) and GitLab's collapsing convention (delegated to python-markdown's own toc extension slugify, already a project dependency, confirmed live to collapse hyphens the same way) against the document's full heading list, with GitHub/GitLab-style numeric -1/-2 duplicate-heading disambiguation applied before matching; falls back to normalized word-token comparison for a slug that doesn't exactly match either scheme. Deliberately does not try to detect which generator produced a given anchor up front — see the design doc for why. Has an inline TODO(#409 follow-up) flagging the cascading try-each-scheme approach as a first pass, not the most precise/efficient possible mechanism. The same QA round (PR #453) found the fuzzy fallback could silently resolve to the wrong heading: it unconditionally stripped a trailing -<digits> from the anchor before comparing, even when those digits were genuine heading wording rather than a dedup suffix, and its match loop returned the first heading with matching normalized tokens with no regard for which duplicate a stripped suffix actually pointed at — directly contradicting the feature's own "a wrong link is worse than no link" design goal. Fixed via _fuzzy_match/_match_tokens_at_occurrence: the anchor's literal tokens (with any trailing digits intact) are tried first at occurrence 0 — safe, since GitHub/GitLab give a duplicate heading's first occurrence the bare/unsuffixed slug, so a suffix-free anchor naming a duplicated heading still resolves correctly without ever needing to strip anything — and only if that fails, and the anchor ends in -<digits>, is the suffix stripped and its number used as the required occurrence index among headings sharing the same normalized tokens; a requested occurrence that doesn't exist (e.g. a claimed 3rd duplicate when only one heading matches) returns None rather than guessing the nearest candidate.

tools/docs/style.py

docs/style.py — helpers (_NAMED_STYLE_TYPES, _read_body_styles, _read_named_styles, _build_named_style_requests, _text_style_and_fields) and tools (style_doc_range, style_doc_table_cells, create_paragraph_bullets, delete_paragraph_bullets, get_doc_theme, get_doc_named_styles, apply_theme). create_paragraph_bullets/delete_paragraph_bullets (#334) wrap the Docs API's createParagraphBullets/deleteParagraphBullets requests, previously only used internally by emitter.py's markdown-to-Doc conversion, as caller-facing tools for fixing list membership/nesting on paragraphs already in a doc — get_doc_structure had no way to even detect a flattened nested list (the AST emitter's own conversion bug from the same issue), and there was no way to fix one afterward short of delete-and-retype as plain text. createParagraphBullets has no nestingLevel field of its own — same constraint the emitter already works around (see that file's own long comment) — the API infers a paragraph's depth purely from its own leading tab characters at call time, consuming them once applied; create_paragraph_bullets reproduces that mechanism per range (insertText-ing nesting_level tab characters before the range, then extending the range's endIndex to cover them) rather than exposing a fake nestingLevel parameter that doesn't correspond to any real request field. get_doc_structure (content.py) now also surfaces each paragraph's bullet field ({listId, nestingLevel} or null), normalizing a top-level item's absent nestingLevel to 0 so every list paragraph reports a level — this is what makes the flattened-list defect (#334's original report) detectable at all, and is the read half create_paragraph_bullets/delete_paragraph_bullets fix on the write side. PR #524's QA round found create_paragraph_bullets's first version didn't work at all on its own flagship use case (nesting paragraphs already in an existing list, #334's original repro): live-verified two compounding causes — createParagraphBullets is a no-op on a paragraph that's already part of a list (doesn't change its level, doesn't consume the signaling tab, which is left behind as literal text), and even after a deleteParagraphBullets+recreate, the API infers a paragraph's depth only relative to whatever else is included in that same call — a paragraph adjacent to an untouched, already-listed neighbor gets silently pulled back to that neighbor's level regardless of its own tabs (the same class of bug PR #432 fixed in emitter.py, confirmed live to also apply here). Fixed by always deleteParagraphBullets-ing a target paragraph first (harmless even if it wasn't previously listed), and — for a paragraph that's already part of a list — expanding the affected span to include its full contiguous run of same-listId neighbors (preserving each neighbor's own current nestingLevel unchanged) before issuing one createParagraphBullets call per contiguous same-preset run, mirroring emitter.py's own established grouping pattern. Confirmed live against the real API for both the multi-paragraph-range case (TC-DOC156) and an isolated single-paragraph target whose only same-list neighbor sits outside the requested range. A second QA round on the same PR caught a follow-on regression from that fix, also live-verified (Playwright): merging context/neighbor paragraphs into the run and applying one bulletPreset to the whole call meant an already-numbered list, repaired without the caller re-passing bullet_preset="NUMBERED_DECIMAL_ALPHA_ROMAN" explicitly, silently converted its untouched siblings to plain bullets — the tool's own hardcoded default (BULLET_DISC_CIRCLE_SQUARE) always won. Fixed by reading the document's own lists map (document["lists"][listId]["listProperties"]["nestingLevels"][level]) to detect an already-listed paragraph's real style before choosing a run's preset — glyphType present means numbered, glyphSymbol present means bulleted, confirmed live for both — and preferring that inferred style over the hardcoded default whenever the caller didn't pass bullet_preset explicitly on any paragraph in the run; an explicit caller-supplied preset still always wins. Since context paragraphs no longer carry a fixed preset up front (it's resolved once per formed run, after grouping), a preset mismatch only forces a run split when it's between two paragraphs the caller explicitly gave conflicting presets to and they're immediately adjacent with no non-explicit paragraph bridging them; a conflict bridged by a shared context paragraph (both explicit ends expand into the same run via a same-listId neighbor between them) is instead reported as an {"error": ...} rather than silently picking one side. Issue #713 (a follow-up filed during PR #711's QA) found create_paragraph_bullets still had the exact nestingLevel 0 collapse emitter.py carried under #439: a formed run in which every paragraph sits at nesting_level >= 1 (no level-0 member — either only such ranges were passed, or the same-listId neighbor expansion pulled in only level-≥1 paragraphs) collapsed to nestingLevel 0 with the depth-0 disc glyph, indent bumped so it still looked nested. Fixed by porting emitter.py's #439 fix verbatim: such a run is wrapped with a throwaway 0-tab anchor paragraph (insertText "\n" at run_start → createParagraphBullets over the range extended by 1 → deleteContentRange the anchor), so the real items land at their true level; the insert (+1) and delete (−1) cancel, leaving positions outside the run untouched and the descending-run application order intact. _text_style_and_fields (shared by style_doc_range and editing.py's insert_softbreak_paragraph) used to clear a hyperlink by setting textStyle.link = {} — the Docs API rejects an empty Link object outright ("must include at least one type"), confirmed live, so link_url: null always failed instead of clearing (#408). Fixed by omitting the link key from textStyle entirely while still naming "link" in the field mask, the correct way to reset a nested message field to its default (no link) per Docs API fields-mask semantics — confirmed live this actually clears an existing link. Both call sites' own if text_style: requests.append(...) guard had to change to if fields: too: a link-clear-only call now legitimately produces an empty text_style dict with a non-empty fields list (["link"]), and the old guard would have silently skipped sending the request at all. Issue #448 factored the omit-key-but-mask-field trick itself out of _text_style_and_fields's inline link_url handling into a small, general _add_or_clear_field(text_style, fields, api_field, value) helper — content.py's heading-anchor-link resolve/strip code (_resolve_heading_anchors) had independently hand-rolled the identical two-line pattern for the same reason, with only a comment pointing back at style.py's prose to explain it; both now call the one shared helper, so a future nullable nested field (e.g. foreground_color: null) has a place to reuse this instead of a third hand-rolled copy, and the "why" lives in exactly one docstring instead of two comments that could drift out of sync.

tools/docs/doc_to_ast.py / docs/ast_to_markdown.py / docs/markdown_export.py

docs/doc_to_ast.py / docs/ast_to_markdown.py / docs/markdown_export.py — the read-side counterpart to the Markdown write path above (issue #300): doc_to_ast.py walks a documents().get() response into the same AST (docs/ast.py) the write pipeline produces (headings, styled runs, nested/ordered/checked bullets via the same document["lists"][listId] glyph-info lookup style.py's create_paragraph_bullets already established, blockquotes inferred from the emitter's own indentStart+borderLeft signal, images via document["inlineObjects"], and tables including nesting via tableCellStyle.columnSpan/rowSpan); ast_to_markdown.py serializes that AST to Markdown, the inverse of html_parser.py's HTML→AST direction; markdown_export.py wires both into the get_doc_as_markdown tool, with an opt-in include_comments param that appends every non-resolved comment (author, quoted anchor, content, replies) via the same Drive comments resource comments.py uses. Deliberately built as a third AST pass rather than converting export_file(export_format='html')'s output through an HTML-to-Markdown library — Google's own Docs HTML export is heavily styled (redundant inline spans), which would need fragile cleanup heuristics to convert cleanly, whereas walking into this project's own AST reuses the exact mapping markdown-support.md already documents, just inverted. Both inline code and a fenced code block are written to the Docs API identically (a run, or every run in a paragraph, with font_family="Courier New" — no other marker), so ast_to_markdown.py treats a Paragraph as a fenced block only when every run in it is code-styled; a paragraph containing nothing but one inline code span (no other text) is genuinely indistinguishable from a one-line code block and renders as a fenced block either way — a documented format-level ambiguity, not a bug (see docs/known-limitations.md's get_doc_as_markdown entry). A nested table (a table inside a table cell) has no Markdown table syntax to express — the mirror image of markdown-support.md's identical write-side gap — so that cell renders a placeholder note instead of silently losing the inner table's content. Live round-trip verification (build a Doc via the existing write pipeline from a Markdown source covering every construct in scope, then read it back and diff) caught two bugs neither the hand-written unit tests nor a docs-API-shape assumption alone would have: a naive f"**{text}**" wrapper breaks CommonMark's emphasis-flanking rule when text has trailing whitespace ("**Bold **" doesn't close, renders as literal asterisks) — fixed by moving leading/trailing whitespace outside the style markers; and joining every block (including consecutive list items) with a blank line produced a needlessly "loose" list — fixed by joining consecutive BulletItem nodes with a single newline instead (CommonMark parses a loose list as the same list either way; this is a readability polish, not a correctness fix). See docs/design/doc-to-markdown.md for the full design writeup. PR #591 QA round 1 found two further narrow silent-content-loss gaps (#594), both in constructs the initial round-trip verification's fixture happened not to exercise: _is_code_block filters node.runs to Run items only before checking font_family, so an Image sitting in an otherwise all-Courier-New paragraph is invisible to the check — the paragraph still correctly renders as a fenced block, but the fenced-block render path then built its text from Run items only, dropping the Image with no trace. Fixed via _render_code_block, which walks node.runs in source order and renders each contiguous run of code text as its own fence with any interleaved Image rendered on its own line outside any fence (Markdown has no way to embed an image inside a fenced block), rather than silently losing whichever Image items were present. Separately, doc_to_ast.py's _cell_content_to_children used to continue past any table-cell paragraph with empty runs entirely — a deliberate blank-line spacer paragraph between two lines of real text (the identical runs=[] shape document_to_ast already produces for a body-level blank line, see test_paragraph_that_is_only_a_newline_becomes_empty_runs) silently merged its neighbors together with a single "\n" joiner instead of two, indistinguishable from a cell with no spacer at all. Fixed by no longer skipping an empty-runs paragraph in the seen_content/joiner bookkeeping: it contributes no text of its own but still counts as content once seen, so the next real paragraph still gets its own separate joiner on top of the blank paragraph's own — preserving the spacer as a second newline (rendered as a doubled <br> in _render_cell's pipe-table output) rather than collapsing it away. PR #599's own QA round 1 (live QA, not just code review) found that round-1 fix was still incomplete at cell edges: doc_to_ast.py already represented a leading or trailing spacer paragraph correctly, as a leading/trailing "\n" on the cell's joined text (confirmed via local AST inspection), but ast_to_markdown.py's _render_cell did text = "".join(parts).strip() before converting "\n" to "<br>" — .strip() silently ate exactly that edge newline, so only a spacer sandwiched between two real lines (the shape round 1's own new tests happened to cover) ever survived to the rendered Markdown. Fixed by reordering _render_cell to convert "\n" → "<br>" first and .strip() second: "<br>" isn't whitespace, so a spacer that's already been turned into a <br> marker survives the strip, while genuine incidental whitespace at the true edges still gets trimmed as before.

tools/docs/html_parser.py

docs/html_parser.py — HTML→AST. _AstParser.handle_data only buffers text while a block context is open (<p>/<li>/heading/table cell) — text with no wrapping block tag was silently dropped (#343). Fixed via a generic _tag_depth counter tracking open non-block tags: bare top-level text (depth 0, e.g. "hello world" with zero surrounding tags) now gets an implicit paragraph wrap, while text merely wrapped in an inline tag with no block ancestor (e.g. <span>no blocks</span>, depth 1) stays an intentional no-op, matching test_inline_only_html_skips_batchupdate. _finalize() (called once after parser.feed() in html_to_ast) flushes whatever block is still open at end-of-input, since the implicit paragraph never gets a real closing tag. _tag_depth excludes tags in _VOID_TAGS (br, img, hr, input, meta, link, area, base, embed, source, track, wbr, col) — Python's stdlib HTMLParser never synthesizes a close-tag call for a void element written without a self-closing slash (e.g. plain <img src="x">), so counting it would leave _tag_depth permanently stuck above 0 and silently re-drop all bare text for the rest of the document (caught in PR #385's own review round — the initial fix only excluded br by name). _emit_block_node (shared by the normal close-tag path, _interrupt_open_block, and _finalize) used to silently drop the entire node — not just the unsupported content — whenever a closed block's buffered runs came back completely empty (e.g. a <p> whose only child was an unhandled <img>), collapsing the paragraph boundary between its neighbors (#401). Fixed via a preserve_if_empty flag: the block's own first/only close now keeps an empty node (runs=[]) instead of dropping it, while a resumed frame's re-close (_block_resumed, set by _resume_interrupted_block) still uses the old skip-if-empty behavior, since an empty result there just means "no trailing text after the nested construct that interrupted this block," not "this tag was empty" — conflating the two reintroduced a spurious empty BulletItem for every <li>text<ul>...</ul></li> (no trailing text after the nested list), caught by the existing test_parent_text_survives_alongside_nested_list regression test. A bare top-level <hr> (python-markdown's rendering of both --- and ___ thematic breaks) never opens a block at all, so it's handled separately in handle_starttag: when no block is currently open, it appends an empty Paragraph node directly rather than falling through the inline-tag no-op path. emitter.py's ast_to_requests makes the matching runs-empty-vs-whitespace-only distinction, since its own if not text.strip(): continue guard would otherwise skip the now-empty node's contribution to full_text and lose the boundary anyway. Whitespace-only paragraphs (e.g. <p> </p> or a lone  ) were deliberately left dropped by that #401 fix as a separate, not-yet-fixed case — fixed under #402: markdown authors commonly write a standalone   line as a deliberate blank-line spacer (a literal blank line collapses between paragraphs), so a freshly-closed <p> (opened and closed with nothing in between, e.g. <p>&nbsp;</p>) now always keeps its whitespace-only-but-non-empty runs, unconditional on preserve_if_empty — there's real (if invisible) content here to lose, not just a boundary. This is deliberately scoped to tag == "p" specifically, not every tag _emit_block_node dispatches on: PR #441's own QA round live-caught a first version of the fix with no such scoping, which also preserved whitespace-only <li>/heading content — <li>Real</li><li>&nbsp;</li><li>More</li> rendered a real bullet glyph with only invisible content between two genuine list items, same for headings — since the blank-line-spacer convention this fix targets doesn't apply to list items or headings the same way. A resumed <p>'s own whitespace-only trailing flush also does NOT get the fresh-<p> treatment: an earlier version of this fix that didn't check self._block_resumed regressed test_nested_list_via_markdown_preserves_parent_text, since a resumed block's trailing whitespace (e.g. the indentation newline python-markdown emits between a nested list's </ul> and the outer <li>'s own </li>) is markup-formatting noise, not authored content. self._block_resumed (tracked since #401 but, per that fix's own history in docs/qa/tests/docs_content.md TC-DOC136, no longer gating anything there — preserve_if_empty/_block_had_unsupported_content replaced it for that purpose) is read again here for this new purpose. Both the non-<p> and resumed-<p> cases fall back to the same preserve_if_empty gate the runs=[] case already used. emitter.py was simplified to match: it no longer special-cases whitespace text at all, just emits whatever text a kept node carries — the empty-string (runs=[]) case still contributes only its trailing "\n". A resumed block's genuine trailing   (not markup-formatting noise) is a known, documented remaining gap — tracked as #444, not fixed here. handle_endtag's <ol>/<ul> close-tag handling used to pop self._list_ordered (the stack BulletItem.depth/.ordered are computed from) only when the closing tag's implied type matched the stack's own top entry — a mismatched pair (e.g. an <ol> closed by a stray </ul>, confirmed live to occur in real-world malformed source HTML) silently skipped the pop instead of performing it, permanently leaving the stack one level too deep for the rest of the document and bleeding a bogus off-by-one depth into every subsequent list, including completely unrelated, later, well-formed ones (#382). _resume_interrupted_block already handles the equivalent node-type correctness concern for this same malformed-tag scenario unconditionally (it's called for both <ol> and <ul> closes regardless of which opened the interrupted block); the fix applies the identical unconditional-pop principle to _list_ordered itself — any </ol> or </ul> means one fewer list is open, popped by count rather than gated on matching the popped entry's own type. Well-formed (correctly matched) nesting is unaffected, since the gate was already true in that case. _resume_interrupted_block itself had the same class of gap one level up (#450, surfaced during PR #449's QA pass on #382 and confirmed pre-existing, not a regression from that PR): it matched a _BlockFrame only by comparing its interrupted_by tag against the exact closing tag, so a mismatched <ol>/<ul> pair left the frame stuck on _block_stack rather than resolving it either way — a later, unrelated, well-formed list could then close with a tag that happened to match the stale frame's interrupted_by value, incorrectly restoring self._block_tag to the original (long-since-closed) outer block and mistyping subsequent bare trailing text as that stale block type. Fixed by giving _BlockFrame a list_depth field (the _list_ordered depth recorded at interrupt time, set only for list-category frames) and matching list interruptions by returning to that depth rather than by tag identity: the frame is always popped once the depth is reached, even on a mismatch, but the outer block is only actually resumed (self._block_tag restored) on an exact tag match — a mismatched close proves the interrupting list ended but never proves the outer block's own close tag was seen, so restoring it would be guessing. This requires _list_ordered's own unconditional pop (the #382 fix above) to run before _resume_interrupted_block for the same closing tag rather than after, since the depth check needs the post-pop count. PR #478's own review round caught a second, related bug in the same function, live-reproduced: bare top-level text directly inside a still-open <ol>/<ul> (not wrapped in its own <li>) opens an implicit paragraph via handle_data's bare-text path (#343) without going through _interrupt_open_block, so it has no frame of its own on _block_stack — resuming an outer interrupted frame while that implicit paragraph was still open unconditionally overwrote self._block_tag and cleared self._run_buf, silently destroying its text (<h1>Start<ol><li>B<ul><li>C</li></ol>D</ol>E dropped "D" entirely and mistyped "E" as a new heading instead of resuming "Start"'s own paragraph). Fixed by flushing whatever block is currently open, exactly the way _interrupt_open_block already does, before actually resuming — a no-op for every other call site into _resume_interrupted_block (table/pre/block-tag closes), since each of those already guarantees nothing is open at that point; only the list-close sites can reach it with something open. The same edit collapsed the function's two branches' duplicate self._block_stack.pop() call into one, per the same review round's non-blocking nit. The <pre> close-tag handler builds its Paragraph directly rather than going through _emit_block_node, so the #401/#402 fresh-vs-resumed whitespace distinction didn't automatically extend to it — a resumed <pre> (interrupted by a nested construct, e.g. a <table>) kept its trailing whitespace-only flush unconditionally, producing a spurious visible-space paragraph (#443). Fixed by applying the same fresh-vs-resumed check directly in the <pre> handler, while still keeping a fresh <pre>'s own whitespace unconditionally (every character inside <pre> is normally significant, unlike <p>); a resumed <pre> that dropped genuinely unsupported content (e.g. a <hr> inside <pre>) still gets a boundary node via the same preserve_if_empty guarantee every other block type gets, caught in PR #515's own QA round. <blockquote> (#476) plugs into the existing interrupt/resume machinery the same way <ol>/<ul>/<table>/<pre> already do — self._blockquote_depth increments/decrements on open/close, and _emit_block_node reads it directly when building each node's blockquote_depth field (a flat integer on Heading/Paragraph/BulletItem/NamedBlock, mirroring how BulletItem.depth encodes list nesting rather than a wrapper node — see docs/design/blockquote-representation.md for the representation decision and why a wrapper node was rejected). PR #546's own QA round 1 found two gaps: three Paragraph-construction sites (bare <hr>, bare <img>, <pre>'s own close handler) built their node directly rather than through _emit_block_node, so each silently defaulted to blockquote_depth=0 — fixed by passing blockquote_depth=self._blockquote_depth explicitly at each site. Separately, _resume_interrupted_block's PR #478 flush-before-resume step (described two sentences above) was only reachable from inside the "a frame exists and will be resumed" branch — a wrapper construct that opened with nothing outer to interrupt (so _interrupt_open_block pushed no frame at all) whose own inner content was left open at close time skipped the flush entirely, confirmed to reproduce identically for plain <ul>text</ul>after with no blockquote involved (a pre-existing gap in the shared mechanism, not something new to blockquote). Fixed by moving the flush to run unconditionally at the top of the function, before the if not self._block_stack: return check — closing the gap for every wrapper construct at once rather than patching blockquote's own manifestation of it.

tools/docs/emitter.py

docs/emitter.py — AST→Docs API requests + multi-phase table fill including nested table and cell styling support. ast_to_requests's BulletItem handling (#336) emits nesting level (BulletItem.depth, correctly computed by html_parser.py but previously never read here) as leading "\t" characters on each paragraph's own text — createParagraphBullets is the only Docs API mechanism for setting a bullet's nesting level, and it works by inferring each covered paragraph's level from, then consuming (removing), that paragraph's leading tab characters at the time the request executes. One call per contiguous same-preset run, not one call per paragraph — confirmed live (PR #432 QA round 1) that issuing a separate createParagraphBullets call per paragraph lets call order/adjacency to an already-bulleted neighbor override the tab-encoded depth: a live documents.get on the result showed sibling paragraphs at the identical AST depth landing under different listIds (each an independent, newly-created list) instead of one shared list with per-paragraph nestingLevel, and ordered lists lost continuous numbering as a result. The fix — confirmed correct via a live round-trip against the real API (shared listId, correct explicit nestingLevel per paragraph, continuous numbering) — groups maximal contiguous runs of same-ordered-value BulletItems (a preset change, e.g. an <ol> nested inside a <ul>, ends a run) into exactly one createParagraphBullets call spanning the whole run, so the API computes every paragraph's level from the tabs within that one call rather than relative to whatever's already rendered from a prior, separate call. A run with no depth-0 paragraph of its own (e.g. a preset-boundary child list, or issue #336's own "bare parent" case — a bare <li> holding only a nested <ul>, whose empty parent <li> html_parser.py drops) is "isolated": createParagraphBullets sets each covered paragraph's nestingLevel to (its leading-tab count) − (the minimum leading-tab count across the whole call's range), so with no 0-tab paragraph the entire run collapses to nestingLevel 0. The original #432 fix left the per-level indent bumped so such a run still looked one level deep, but it rendered the depth-0 glyph (● disc) instead of the correct nested glyph (○ circle at depth 1) — issue #439. Now fixed: an isolated run is wrapped with a throwaway 0-tab anchor paragraph — insertText "\n" at run_start, createParagraphBullets over the range extended by 1, then deleteContentRange the anchor — so the real items land at their true nestingLevel and its glyph (confirmed live, any depth: a min-depth-2 run resolves to nestingLevel 2). This is deliberate, not an accident of the anchor mechanic: because the anchor always contributes 0 tabs, nestingLevel for every item in an isolated run always equals its own depth exactly, with no clamp — a min_depth == 2 fragment (e.g. two levels of empty parent <li>s dropped, leaving only depth=2 children) renders two indent tiers deep with no visible parent above it. Issue #714 confirmed this is the intended behavior over clamping to depth 1: the converter renders the source's true structure faithfully rather than second-guessing a fragment whose ancestors were dropped only for having no text of their own. The anchor insert (+1) and delete (−1) cancel, so every position outside [run_start, run_end] — later bullet runs, the insertTable/image pass — is left exactly where an ordinary single createParagraphBullets call would leave it, and the descending-document-order application still holds (a lower run's net-zero anchor pair plus tab consumption never touches a higher, not-yet-processed run's range). Each bullet_run_requests entry is therefore a list of requests, not a single request. Because each grouped call still consumes tab characters (shifting every later document position backward by the run's total tab count), it can't run inline with the rest of the per-node request loop (which otherwise assumes no position ever shifts, true for every other request type) — it's deferred into a separate pass appended after all other per-node requests but before the insertTable pass, applied latest-document-position-first so consuming one run's tabs never invalidates the still-to-run range of an earlier, not-yet-processed run. insertTable positions are computed the same way they always were (as if the tabs were still present, since that's the coordinate space they were captured in while walking nodes) and then decremented by the cumulative tab count of every run positioned before that table, since by the time insertTable actually runs, all of those tabs have already been removed. A document with no nested (depth>0) bullets produces exactly the same one-call-per-contiguous-run shape it always would have, just now also merging what used to be separate same-preset flat-list calls into one — harmless, since a multi-paragraph call with zero tabs anywhere still assigns every paragraph level 0. Blockquote content (#476, any node with blockquote_depth > 0) gets one additional updateParagraphStyle request setting indentStart (36pt × depth) and borderLeft (a constant gray solid line) — the closest visual equivalent to a real blockquote the Docs API's ParagraphStyle exposes, since there's no native blockquote namedStyleType. borderLeft was live-verified writable via a scratch-doc round trip before implementing (per this file's own "Verify a ticket's API premise live" rule) rather than trusted from the discovery schema's silence on read-only status alone — the same class of field tabStops (#404) turned out NOT to be despite looking identical on paper. Issue #719: ast_to_requests used to join a code-block Paragraph's Run text (which legitimately contains embedded "\n" line breaks for a multi-line fenced block, per html_parser.py's <pre> handling) straight into the shared insertText call — but the Docs API treats every "\n" in inserted text as a new paragraph boundary regardless of styling, so a single multi-line code block silently fragmented into one NORMAL_TEXT Docs paragraph per line, contradicting markdown-support.md's own documented mapping of a fenced block to one "monospace paragraph" (confirmed live both ways: creation still applied Courier New to every fragment correctly, but get_doc_as_markdown's round-trip then re-rendered each fragment as its own tiny separate fence instead of one — reproducing identically for plain code content, not just HTML-tag content despite the issue's original title). Fixed via _is_code_paragraph (mirrors ast_to_markdown.py's _is_code_block, duplicated rather than imported to avoid a write/read circular import): a code-styled Paragraph's internal "\n" breaks are substituted with "\v" (the same soft-line-break convention insert_softbreak_paragraph already uses) before insertion, keeping the whole block one Docs paragraph while still rendering each line on its own visual line; the node's own trailing separator "\n" to the next AST node is untouched. doc_to_ast.py's _text_run_to_run reverses the substitution ("\v" → "\n") for any run whose weightedFontFamily is Courier New specifically, so the reversal can't misinterpret an unrelated genuine soft break in non-code content — which currently has no reverse conversion of its own and round-trips as a literal "\v" character, a separate, pre-existing gap left out of scope here. Issue #434 (7 non-blocking findings from /code-review high on PR #432, never previously acted on) was resolved, across two PR #768 rounds, as: finding 1 fixed — a literal leading tab in a BulletItem's own source text (indistinguishable from the synthesized depth-tabs once concatenated) is stripped from its joined run text before combining with tabs, since left in place it both nests the item one level too deep and silently under-counts cumulative_tabs, desyncing any later table/image position (TC-DOC193); finding 3 fixed — an orphan <li> outside any <ul>/<ol> (node.depth would otherwise be -1) is clamped to a minimum of 0 at the actual source, html_parser.py::_make_bullet_item's depth = len(self._list_ordered) - 1, rather than by 3 separate downstream emitter.py call sites each independently remembering to — so it takes the same 1-request min_depth == 0 path a normal depth-0 item does instead of the 3-request isolated-wrap path purely because of an unclamped sign (TC-DOC194, unit-test-only; both converge on the identical rendered nestingLevel 0, so this is an efficiency/consistency fix with no observable output change); finding 4 fixed alongside 3 — the 3 now-redundant max(node.depth, 0) clamps in emitter.py were removed once the source-level clamp made them unreachable; findings 5+7 fixed — the three separately-appended parallel lists (tables/table_positions/table_tab_offsets) were consolidated into one pending_tables: list[_PendingTable] (a small local dataclass), and the previously-inlined table_positions[i] - table_tab_offsets[i] position correction is now a named position variable at its one call site, closing finding 7 as a byproduct; finding 6 fixed — ast_to_requests's docstring now documents all 3 ordering-sensitive request stages, and the previously-untyped positional segment_meta 5-tuple became a typed _SegmentMeta dataclass, mirroring _PendingTable's own rationale; finding 2 was already moot — the cited cumulative_tabs += len(tabs) never actually shipped that way (PR #432's own merged commit already used utf16_len(tabs), confirmed via git show); finding 4's original ask (generalizing the deferred-request mechanism into a reusable "this request consumes/shifts positions" abstraction, framed by its own filing as speculative future-proofing rather than a described defect) was deliberately left unimplemented, per this repo's standing guidance against designing for hypothetical future requirements — note this is a different "finding 4" than QA round 2's own locally-numbered findings, described next. PR #768's own QA round 1 (Kit) caught that the first version of finding 1's fix only stripped the leading tab from the concatenated run_text string used to build the inserted text, not the underlying Run/Image objects the per-run style/position offset loop iterates over afterward — a styled run's updateTextStyle range, and an Image's insertInlineImage position, both still counted via utf16_len(item.text) on the unstripped original text, overrunning by however many tab characters were actually stripped but never inserted (live-confirmed: a depth=0 BulletItem(runs=[Run('\tHello', bold=True)])'s own bold range came back {startIndex: 1, endIndex: 7} when "Hello" — the only text actually inserted — occupies indices 1-6). Fixed by extracting the strip into _strip_bullet_leading_tabs, a pure, non-mutating helper (returns a new Run/Image sequence via dataclasses.replace, never touching the caller's own AST objects — this function must stay side-effect-free on caller-owned input, unlike html_parser.py::_make_bullet_item's in-place runs[0].text = ... mutation, which is safe only because it always operates on a list it just built from its own fresh buffer) and threading that stripped sequence through as _SegmentMeta.runs, which the offset loop now walks instead of node.runs directly. The same QA round also flagged that the strip's own content-loss tradeoff (a genuinely-authored leading tab, not just hand-formatting whitespace, is silently dropped — unconditional on depth, since even a depth-0 item's own literal leading tab is still misread as a nesting signal relative to the rest of its createParagraphBullets call) had no test covering depth 0 specifically and no comment calling out that it is content loss, not just a reinterpretation; both addressed with an explicit callout in the strip's own comment and a dedicated depth=0 regression test.

Async execution

Async execution (execute_in_thread + thread_http, in http_transport.py): every @tool()-registered function is async def, and _timed's wrapper is unconditionally async def wrapper(...): return await func(...) — so any new tool must be async def, even one with no Google API calls (see tools/cache.py's get_cache_ttl/set_cache_ttl/refresh_cache for an example with an empty await-free body; a plain def there raises TypeError at call time since awaiting a sync return value fails — test_server.py::TestAllToolsAreAsync guards against this recurring). Every .execute() call is wrapped await execute_in_thread(chain.execute, service) (http_transport.py, re-exported from auth.py for existing imports) — do not write asyncio.to_thread(chain.execute, http=thread_http(service)) directly: thread_http(service) as an eagerly-evaluated kwarg resolves on the event-loop thread before the worker thread starts, so concurrent calls end up sharing one transport across threads (a real, live-reproduced bug, fixed in #183's follow-up round — see the decision doc). execute_in_thread defers the thread_http() call into the thread itself via a lambda. thread_http() hands each worker thread its own AuthorizedHttp transport built from the shared credentials, since the lifespan-scoped sheets_service/drive_service/etc. objects each carry one httplib2 transport that isn't safe for concurrent use across threads. Six multi-call tools (sync_folder, get_multiple_sheet_data, get_multiple_spreadsheet_summary, import_csv_to_sheet, share_spreadsheet, share_file) parallelize their per-item work via asyncio.gather(..., return_exceptions=True) instead of looping serially. cache.py's SQLite I/O deliberately stays synchronous, not to_thread-wrapped — see the comment in cache.py::_open(); the module's two Google-API-calling helpers (get_modified_time, fetch_sheets) are async def and do use execute_in_thread for their own .execute() calls. _BaseCache.snapshot_epoch()/store(..., epoch=...) guard against a refresh_cache() call landing mid-fetch and having its invalidation silently overwritten — wired up for SheetStructureCache/SheetDataCache/DriveFolderCache (not DocContentCache/CalendarCache), and scoped per key: a store is skipped only if an invalidation covering its own key, <id>: prefix, or the whole namespace landed after its snapshot (TTL expiry doesn't count). Size-bounded caches (max_results/rows_to_fetch) share _BaseCache._get_sized/_store_sized rather than re-implementing the rows_fetched sufficiency check (#698). Full rationale: docs/decisions/decision-async-tool-execution.md. When unit-testing genuine cross-thread concurrency (not just correctness) with a threading.Barrier or similar, the block must live inside the mocked .execute() call, not .list()/.create()/etc. — those are evaluated eagerly on the event-loop thread while building the call chain, before execute_in_thread ever hands off to a worker thread, so blocking there freezes the single-threaded event loop instead of proving concurrency (see tests/drive/test_transfer.py::TestSyncFolderRecursive::test_recursive_sibling_subfolders_descend_concurrently). download_folder/sync_folder (#316/#319) added ctx.report_progress() calls from inside each item's own per-item coroutine so updates stream as concurrent transfers complete rather than arriving in one burst after the whole gather resolves — any future tool adding progress reporting to a gather-based batch (tracked for the remaining asyncio.gather tools in #355) must wrap that report_progress call in its own try/except: it's awaited after the item's real work already succeeded or failed, with no other guard around it, so a report_progress exception (e.g. a dropped client session) would otherwise propagate out, get caught by the outer gather(..., return_exceptions=True), and overwrite an already-successful result with a spurious failure — confirmed live and fixed in PR #351's review round.

Sheets API TextFormatRun.startIndex is also UTF-16 code units

Sheets API TextFormatRun.startIndex is also UTF-16 code units (confirmed via the API reference, same gotcha class as the Docs API note above): update_cells (tools/sheets/data.py) accepts a rich-text cell as a list of {"text": str, "hyperlink": str | None} run dicts for partial (rich-text) hyperlinks within a single cell (issue #89) — _build_rich_text_cell computes each run's startIndex via _utf16_len (sheets/helpers.py, which delegates to docs/indices.py's utf16_len rather than duplicating its per-character logic), not len(), so a run following an astral-plane character (emoji, some CJK/math symbols) lands at the correct offset. A data array with no rich-text cells still goes through the original whole-range values().update() USER_ENTERED path unchanged. A data array that mixes plain and rich-text cells does not — an earlier version wrote the whole range via values().update() with rich-text positions blanked to "", relying on the second batchUpdate pass (the one that writes the rich-text updateCells requests) to overwrite those blanks back; if that second call failed, the blank was never restored and the rich-text cells' prior content was lost (live-confirmed via TC-W34 review, PR #380). The fixed version writes only the actual plain-cell positions via a per-cell values().batchUpdate() instead, so the two passes never touch each other's cells and a failure in one can't corrupt the other's data. Because they're now two independent API calls whenever both plain and rich-text cells are present, the tool returns {"values_update": ..., "rich_text_update": ...} in that case rather than silently dropping one result — only a call with just one kind of cell returns that single call's raw response.

MCP resources

MCP resources: Two resources are registered — server://auth-status (active auth method and Drive capability limits) and spreadsheet://{spreadsheet_id}/info (sheet list and grid properties). Under mcp v1, neither took a ctx argument the way tools do — both fetched it explicitly via mcp.get_context().request_context.lifespan_context (FastMCP had no get_lifespan_context() method, confirmed absent as far back as mcp==1.27.1, so calling that nonexistent method was never a regression from #350's SDK bump; it was a pre-existing bug invisible to unit tests since TestAuthStatusResource only exercised _auth_status_json() directly, never the resource function itself — fixed in #363). Under mcp v2 (mcp.server.mcpserver.MCPServer, since #175), get_context() no longer exists at all, and Context injection for a static (non-templated) resource like server://auth-status isn't supported by the framework (confirmed live against mcp==2.0.0 — the decorator raises ValueError outright). The two resources now use different mechanisms accordingly: get_spreadsheet_info is a template resource ({spreadsheet_id} in its URI), where v2 does support Context injection the same way tools already get it — it takes ctx: Context as an ordinary parameter and reads ctx.request_context.lifespan_context. get_auth_status is static, so it reads the process-wide auth.get_lifespan_context() singleton described above instead — see docs/decisions/decision-mcp-v2-resource-context-access.md for the full rationale.

MCP restart

MCP restart: After docker compose restart mcp-gee-sweet, Claude Code does not automatically reconnect to the SSE server — you must restart Claude Code too to re-establish the connection. The same staleness applies to a team role's own stdio mcp-gee-sweet-<name> server (confirmed while fixing #336): it's a single long-running subprocess that imports this package's code once at session start, so editing .py files afterward does not get picked up by that already-running process — a live tool call (e.g. create_doc) made after a mid-session edit silently exercises the pre-edit code, which can look exactly like a real live-rendering bug (it reproduced one that had already been fixed on disk). uv run python3 -c "..." invocations in the same worktree are unaffected (fresh interpreter, fresh import each time) and are the reliable way to live-verify a same-session code change against the real API without restarting the whole session — reuse mcp_gee_sweet.auth._oauth_creds() plus googleapiclient.discovery.build(...) directly (the same construction spreadsheet_lifespan uses) to get a real docs/drive service in a throwaway script. This is also the only way to inspect fields no tool surfaces, like documents().get()'s raw paragraph.bullet/lists — what actually pinned down PR #432's live nesting behavior.

Regression vs. pre-existing bug after a dependency bump

Regression vs. pre-existing bug after a dependency bump: when a bug report asks "is this a regression from the recent SDK bump" (e.g. issue #363 against #350's mcp>=1.28.1 change), don't answer from memory or by reading changelogs alone — install the old pinned version in a scratch venv and check the specific API surface directly against it (e.g. python3 -m venv /tmp/x && ./x/bin/pip install "mcp==1.27.1" && ./x/bin/python -c "from mcp.server.fastmcp import FastMCP; print(hasattr(FastMCP, 'get_lifespan_context'))"). Confirmed in #363: FastMCP.get_lifespan_context() looked like it could've been removed by the bump, but it never existed even at 1.27.1 — the scratch-venv check settled the question in seconds instead of guessing from a diff.

Verify a ticket's API premise live before implementing, not after

Verify a ticket's API premise live before implementing, not after. An issue's ask can rest on an unverified assumption that a specific Google API field or request is settable, just because that field appears in the API's discovery schema — a schema documents read-only fields as ordinary properties, so presence alone doesn't mean writable. Before building a feature around a Google API capability not already confirmed live elsewhere in this codebase, run the same scratch-script check described above under "MCP restart" (uv run python3 -c "..." with mcp_gee_sweet.auth._oauth_creds() + googleapiclient.discovery.build(...)) against the live discovery document first — read the target field's own description for a "read-only" note, and confirm at least one request type in the schema actually references it for writing — before writing any implementation. Confirmed the hard way on issue #404/PR #471: style_doc_range shipped a full tab_stops implementation, unit tests, and QA test cases before QA's live round hit HttpError 400 ... "Unallowed field: tabStops". A follow-up discovery-schema check took two minutes and confirmed ParagraphStyle.tabStops's own description reads "This property is read-only," with no request type anywhere in the Docs API able to set it — the same check, run before implementation instead of after QA sent it back, would have caught this without costing a full dev+QA cycle. Closed as infeasible rather than fixed (issue #404, PR #471, follow-up #473 filed for the documented workaround).

Procedural feedback → the governing command file, not memory

Procedural feedback → the governing command file, not memory. If the human's feedback corrects or refines a repeatable team-workflow step that a .claude/team-roles/*.md or .claude/commands/*.md file already governs (or should) — as opposed to project context, an external-system pointer, or a one-off judgment call specific to the current conversation — that's a command decision (.claude/commands/retro.md's disposition), not a memory entry: edit the file directly, through Bob's prompt-QA gate (.claude/team-roles/bob.md), rather than trusting memory to carry it. A memory-only fix is retrieved heuristically and competes with whatever the loaded prompt already says instead of reliably overriding it — confirmed the hard way on PR #456: two separate memory-only fixes for QA's /mcp reconnect+/code-review batching (qa.md step 2) each got silently re-broken by a later session before the rule was hard-coded into qa-kickoff.md instead. Memory is still the right home for what's genuinely outside the team's tracked process — it just isn't a substitute for a rule a command file is meant to enforce every time.