docs: check timezone handling against the contributor guide in the PR review skills

[andygrove]

Which issue does this PR close?

No issue. This follows #6337, which added the timezone handling page to the contributor guide, and is part of the timezone EPIC #6335.

Rationale for this change

#6337 added docs/source/contributor-guide/timezones.md, but the review skills don't point at it. A review of a datetime expression, a cast, a scan or the JVM/native boundary has no reason to open the page.

Most of the bugs in #6335 are ones a review could have caught:

• timestamp_seconds returns a TimestampType result with no timezone label (Native timestamp_seconds returns a TIMESTAMP_NTZ-typed array, so downstream expressions ignore the session timezone #6328)
• date_trunc stamps the session timezone on its result, so comparisons fail in Etc/UTC sessions, where it runs natively (Native date_trunc labels its output with the session timezone, so comparing it with another timestamp fails in Etc/UTC sessions #6330)
• CometDays reads SQLConf.get.sessionLocalTimeZone instead of the timezone on the expression (days transform is evaluated in the session timezone, while hours and Iceberg use UTC #6333)

These pass any test that only projects the result.

No single area skill owns timezone handling. It spans the serde, the native kernels, the scans, the JVM/native boundary and the codegen dispatcher. So the main check goes in review-comet-pr, and the area skills get short items for the parts that are specific to them.

What changes are included in this PR?

• review-comet-pr: step 1 says to read timezones.md for any PR that deals with timestamps or the session timezone. Step 5 gets a "Timestamps and timezones" check. It covers how to tell whether a PR is affected (including a grep over the diff), the problems to look for first, and which parts of the page a PR can make stale.
• review-comet-expression-pr:
  • The page goes in the guide table.
  • New checklist items cover serializing expr.timeZoneId through CometTimeZone.nativeId, labelling TimestampType results "UTC", DataFusion datetime functions that evaluate in UTC, and UTC fast paths.
  • The timezone test item now follows the page's testing section.
  • "Timestamp result mislabelled" is added to the common findings.
• review-comet-ffi-pr: timestamps cross the boundary unconverted, and a new JVM producer labels them with CometArrowStream.NATIVE_TIMEZONE.
• review-comet-iceberg-write-pr: timestamp partition values are UTC. PartitionValueCalculator computes them with the same kernels that key the sort in front of a clustered write, so a change that moves a time transform back to iceberg-rust, whose years and months follow the column's timezone label (Year and Month transforms depend on the input array's timezone tag; Iceberg computes them in UTC iceberg-rust#3142), breaks that agreement.

This should merge after #6686, which brings the page up to date with the fixes from #6335. The skills describe the page as it reads after that PR, for example how CometTimeZone rewrites timezone IDs.

How are these changes tested?

Documentation only. prettier --check passes on the changed files. I checked every name the skills cite against main: CometArrowStream.NATIVE_TIMEZONE, CometTimeZone.nativeId and supportLevel, array_with_timezone, is_utc_timezone in extract_date_part.rs, the Iceberg transform kernels, and PartitionValueCalculator. I also tried the diff search on recent commits. It matches three datetime commits, and it doesn't match three shuffle commits or #6250.

[sunchao]

Summary

• Prior state and problem: The review skills lacked explicit checks for timestamp labels, expression timezones, and timezone-sensitive tests.
• Design approach: Centralize the shared checks in review-comet-pr, with focused additions for expressions, FFI, and Iceberg writes.
• Correctness / compatibility analysis: The guidance agrees with the inspected Spark 3.4/3.5/4.0/4.1/4.2 sources, Comet’s timestamp representation, and the pinned Iceberg implementation. Expression timezone selection and Iceberg’s normalization before partitioning are consistent with the new checks.
• Key design decisions: Distinguish the "UTC" storage label from the expression’s timezone, preserve TimestampNTZType semantics, and test results through downstream expressions. The existing skill structure accommodates these checks without a new abstraction.
• Implementation sketch: Four Markdown skill files change. There are no runtime changes or added query-time overhead.
• Behavioral changes worth calling out: The acknowledged prerequisite #6337 remains open. docs/source/contributor-guide/timezones.md is absent at this SHA, confirmed by git cat-file -e. The new mandatory reading and testing references therefore cannot be followed from this checkout. This existing merge-order blocker remains unresolved.
• Suggested improvements: No new introduced P1/P2 issues found within this review. Resolve the already documented prerequisite before merging.

Reviewed the entire diff from e897f8ab45dec8fc27c79339c65eeaa7bd60e198 to ad8228bfded01262ca23b8b07740de98da08aaa0. The PR is not a draft. There were no existing reviews, issue comments, inline comments, or review threads.

Routed skills: review-comet-pr, review-comet-expression-pr, review-comet-ffi-pr, and review-comet-iceberg-write-pr.

Exact-head CI: check-pr-title and label passed. Preflight and Analyze Actions remain in progress. No completed check failed.

Validation: git diff --check passed. Checked changed references and relevant Comet, Spark, DataFusion, and Iceberg sources. Consulted #6337’s proposed guide as external context. No JVM/native builds or runtime suites were run for this documentation-only diff. The working tree remains unchanged.

[sunchao]

Summary

• Prior state and problem: The review skills did not direct reviewers to the timezone guide or consistently check timestamp labels and expression timezones.
• Design approach: Add shared guidance in review-comet-pr, with focused checks for expressions, FFI, and Iceberg writes.
• Correctness / compatibility analysis: The additions agree with the inspected Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0 sources, Comet’s timestamp representation, and the relevant Iceberg implementations. They correctly distinguish the "UTC" storage label from an expression’s evaluation timezone.
• Key design decisions: Check declared and produced timestamp types, preserve TimestampNTZType semantics, exercise multiple timezones, and consume results beyond projection. Iceberg partition checks correctly reference the shared Comet kernels and Java rounding behavior.
• Implementation sketch: Four Markdown skill files extend the existing review structure. There is no new runtime abstraction or query-time overhead.
• Behavioral changes worth calling out: Comparison with branch-1.1 shows intended review-guidance changes, with no execution behavior changed by this PR. The earlier missing-guide blocker is resolved. Prerequisites #6337 and #6686 have merged.
• Suggested improvements: No introduced P1/P2 issues found within this review. No substantiated existing P1/P2 concern remains unresolved.

Reviewed the entire two-commit diff from ba08acd815d5fd15f75ce9f314715778b9161cf1 to 0a70c4936f955f0af7fb128d5e6718444b9b9724. The PR is not a draft. Read the supplied discussion snapshot, including the prior review. It contains no issue comments, inline comments, or review threads.

Routed skills: review-comet-pr, review-comet-expression-pr, review-comet-ffi-pr, and review-comet-iceberg-write-pr.

Exact-head CI: Seven checks passed, including Required Checks, Preflight, and CodeQL checks. Sixteen checks were skipped, including Linux/macOS builds and Spark/Iceberg suites. No checks failed or remain in progress.

Validation limits: git diff --check passed, added guide references resolve, and relevant Comet, Spark, DataFusion, and Iceberg sources were inspected. Prettier could not run because npx is unavailable. No JVM/native builds or runtime suites were run for this documentation-only diff. The working tree remains unchanged.