[WIZARD221]

Description

Adds an opt-in UPDATE_GOLDEN=1 environment flag so the schema golden-file tests can regenerate their snapshots, instead of forcing contributors to hand-edit the JSON.

Why this is needed

Two tests assert a generated tool schema against a committed snapshot and fail on any drift:

• tests/gcontacts/test_contacts_tools_v2.py → golden/contacts_tool_schemas.json
• tests/gdocs/test_strikethrough.py → golden/docs_tool_schemas.json

These guards are valuable — they catch accidental changes to a tool's public contract. But there was no supported way to update a snapshot after an intentional schema change. Any PR that legitimately changes a contacts/docs tool schema (adding a field, a parameter, etc.) makes these tests fail, and the only recourse is to manually reconstruct the exact json.dumps(..., indent=2, sort_keys=True) output by hand — easy to get subtly wrong, and the failure message gave no guidance.

This was hit in practice while adding a birthday field to the contacts tool: the schema-golden test failed and had to be regenerated manually.

What this does

• Adds a shared helper tests/golden_utils.assert_matches_golden(generated, golden_path, label).
• UPDATE_GOLDEN=1 → the helper rewrites the golden file (canonical indent=2, sort_keys=True + trailing newline) and returns.
• Default (unset) → it compares and fails on drift exactly as before, now with a hint:

  If this change is intentional, regenerate the snapshot with:
      UPDATE_GOLDEN=1 uv run pytest
  

• Refactors both existing golden tests onto the helper. The per-test structural assertions are unchanged.

Workflow for an intentional schema change becomes:

UPDATE_GOLDEN=1 uv run pytest    # rewrites snapshots
git diff                         # review the schema change deliberately

This is the standard snapshot pattern (pytest-snapshot's --snapshot-update, Jest's -u, Rust insta's INSTA_UPDATE). The diff still surfaces in code review, so the guardrail isn't weakened — it just stops being a manual transcription chore.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update / developer tooling (test-only; no runtime change)

Testing

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have tested this change manually

Verified locally:

• Default mode: both golden tests pass unchanged (90 passed for the two files; 1199 passed, 2 skipped full suite).
• UPDATE_GOLDEN=1 regenerates the snapshots byte-identically when the schema is unchanged (no spurious diff).
• Drift detection still works: perturbing a golden file fails the test in default mode and prints the UPDATE_GOLDEN hint; rerunning with the flag restores it.
• ruff check and ruff format --check pass on all changed files (no unused imports left behind).

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have enabled "Allow edits from maintainers" for this pull request

Additional Notes

Test-only change — no runtime/production code is touched. Independent of (and complementary to) the birthday feature in #833, which is what surfaced the gap.

Summary by CodeRabbit

• Changes

  • Updated contact tool schemas with expanded structured fields for organizations, nicknames, URLs, user-defined fields, and relations.
  • Added backward-compatible value aliases for email and phone inputs.
  • Clarified batch operation inputs by separating contacts, updates, and contact IDs.
  • Legacy scalar contact fields remain marked as deprecated.

• Tests

  • Added shared golden-file comparison utilities and updated schema tests to use them.

• Documentation

  • Clarified the process for reviewing and intentionally regenerating schema snapshots.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a0fc323d-ac35-405a-a08c-e9fd7fe2ec6b

📥 Commits

Reviewing files that changed from the base of the PR and between 31036c0 and 8ef55e2.

📒 Files selected for processing: 4

• .github/instructions/general.instructions.md
• tests/gcontacts/golden/contacts_tool_schemas.json
• tests/gcontacts/test_contacts_tools_v2.py
• tests/golden_utils.py

🚧 Files skipped from review as they are similar to previous changes: 2

• .github/instructions/general.instructions.md
• tests/gcontacts/test_contacts_tools_v2.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Adds a shared golden-file assertion helper, updates the contacts schema snapshot, migrates Contacts and Docs schema tests to the helper, and documents the UPDATE_GOLDEN=1 uv run pytest regeneration workflow.

Changes: Golden Schema Contract Updates

Layer / File(s)	Summary
Contacts schema snapshot updates tests/gcontacts/golden/contacts_tool_schemas.json	Updates contact, batch, structured-field, deprecated-field, and operation schema definitions.
Golden assertion utility tests/golden_utils.py	Adds canonical JSON serialization, optional golden-file rewriting through UPDATE_GOLDEN=1, and unified-diff failure reporting.
Schema test migration and instructions tests/gcontacts/test_contacts_tools_v2.py, tests/gdocs/test_strikethrough.py, .github/instructions/general.instructions.md	Replaces duplicated golden comparison logic with assert_matches_golden and documents the snapshot regeneration command.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Suggested reviewers: taylorwilsdon

Merge Risk: ⚪ Minimal · up to 8ef55

The snapshot workflow is ready to merge after normal checks; the proposed key sorting would change its intended format.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: adding an UPDATE_GOLDEN flag to regenerate schema snapshots.
Description check		The description includes all required sections, explains the change and rationale, identifies the test-only scope, documents testing results, and completes the checklist.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/golden_utils.py`:
- Around line 43-46: The golden file I/O uses pathlib.Path.write_text/read_text
without an explicit encoding, which can vary by host locale; update the calls
around golden_path.write_text(_serialize(generated)) and golden =
json.loads(golden_path.read_text()) to pass encoding="utf-8" (i.e.,
golden_path.write_text(_serialize(generated), encoding="utf-8") and
json.loads(golden_path.read_text(encoding="utf-8"))) so reads/writes are
deterministic across environments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f18a10c-38ef-462f-9601-21536b8d496a

📥 Commits

Reviewing files that changed from the base of the PR and between a069da4 and fc55c38.

📒 Files selected for processing (3)

• tests/gcontacts/test_contacts_tools_v2.py
• tests/gdocs/test_strikethrough.py
• tests/golden_utils.py

[taylorwilsdon]

Thanks, this is a nice quality of life one! Merged main in and took a cleanup pass, one catch since you opened it - #1230 rewrote docs_tool_schemas.json in generation order instead of sorted, so the first UPDATE_GOLDEN=1 run reordered ~830 lines of the docs golden and buried the actual change. Switched the helper to write generation order (same order the model sees) and compare the serialized text so a hand-edited golden can't quietly drift out of format again. That means a one-time reorder of the contacts golden, docs is untouched. Will get this in the next release!