Chapter 19 Flashcards — Critique: Google’s Code Review Tool
flashcards seg critique code-review tools
What is Critique?
?
Critique is Google’s internal web-based code review tool, used to review Change Lists (CLs) before they are submitted to the repository. It manages the entire CL lifecycle: from diff rendering and automated analysis surfacing at creation, through reviewer selection, commenting, approval tracking, and post-commit record archiving. Critique’s design actively shapes review behavior rather than being a passive medium.
What are the six stages of the code review lifecycle in Critique?
?
- Create a change — author creates a CL; Critique renders the diff and runs automated analysis 2. Request review — author selects reviewers; tool suggests ownership-based candidates 3. Understand the change — reviewer reads the diff with analysis findings inline 4. Comment on the change — reviewer drafts and publishes inline comments 5. Change approvals — CL collects LGTM, owner approval, and readability approval 6. Commit the change — final presubmit checks pass; CL is submitted; record is archived
What happens when an author creates a CL in Critique (Stage 1)?
?
Critique performs several functions immediately: renders a diff with enough surrounding context for reviewers to understand changes; runs Tricorder and other analyzers whose findings appear inline in the diff at the relevant lines; shows build and test status (whether the CL compiles and tests pass); and surfaces one-click suggested fixes for analysis findings that have automated remediations. The author sees all this before even requesting review.
What is the purpose of draft comments in Critique?
?
Draft comments are comments the reviewer has written but not yet published. They allow a reviewer to compose a complete review — reading the entire diff and annotating every issue — before sending any feedback. When the reviewer publishes, the author receives one coherent batch of feedback rather than a stream of individual notifications. This reduces the author’s context-switching cost and makes review rounds more efficient.
What does Critique’s explicit review state show?
?
Critique maintains and displays explicit state for every reviewer: Unreviewed (not yet engaged), Reviewing (CL is open, reviewer is working through it), and Reviewed (pass complete). This state is visible to the author. It creates social accountability — an author can see if a reviewer has been in “Reviewing” for a long time without posting — and eliminates the uncertainty of “has anyone started looking at my change?”
What are the three approval gates a CL must pass before it can be submitted?
?
- LGTM — granted by any reviewer familiar with the code; attests to correctness and design quality. 2. Owner approval — granted by a designated OWNER of the affected code paths; attests that the change is appropriate for this codebase. 3. Readability approval — granted only by an engineer certified in the language’s readability process; attests that the code follows Google’s idiomatic style. A single reviewer may satisfy multiple gates if they hold the required ownership and certifications.
Why does Critique automatically retract an LGTM when an author makes significant changes after approval?
?
To prevent the sneak-change anti-pattern: an author receives approval and then makes additional changes — potentially significant ones — that the approving reviewer never sees. Auto-retraction forces the reviewer to re-examine the updated CL before submission, ensuring that every approval was granted with full knowledge of the final change. This is a tool-enforced safeguard for a failure mode that social norms alone cannot reliably prevent at scale.
How does Critique integrate Tricorder’s static analysis findings into the review experience?
?
Tricorder findings appear as inline comments in the diff at the exact lines they pertain to, visually distinguished from human comments. This eliminates the context-switch required if analysis results were in a separate report — reviewers see automated and human observations in the same view. For findings with confident remediations, a one-click suggested fix button allows the fix to be applied directly without manual editing. Dismissed findings feed back as training signal to the analyzer.
Why is inline surfacing of analysis results more effective than a separate analysis report?
?
When analysis results are in a separate report, engineers must context-switch out of the review, open the report, read findings without the code in view, then mentally reconnect each finding to the code. In practice, this friction causes most findings to be ignored. Inline surfacing eliminates the context switch entirely — the reviewer reads the diff and encounters automated findings in the same view, with the relevant code immediately visible. The cognitive distance from “seeing a finding” to “acting on it” is minimal.
What is the purpose of Critique’s post-commit record?
?
After a CL is submitted, Critique permanently archives the full record: all comments (human and automated), all revisions, all approval signals, and the final submitted diff. This record serves as: a debugging artifact (investigators can see what changed and why), a knowledge artifact (CL discussions contain design rationale not captured elsewhere), and an accountability record (who approved what is permanently on record). CLs become searchable institutional knowledge, not ephemeral events.
What is the notification design principle at the core of Critique?
?
Over-notification destroys the value of notifications. If engineers receive a notification for every comment, revision, and automated finding, they learn to ignore them — and then miss genuinely important ones. Critique is designed so notifications are high-signal and action-required: authors are notified when reviewers post comments or approve/reject; reviewers are notified when authors update the CL. Comments are batched per review round. Analyzer findings do not generate separate notifications by default.
How does Critique help authors select appropriate reviewers?
?
Critique analyzes the files changed in a CL and suggests reviewers based on OWNERS file declarations — the code ownership system that designates responsible engineers for each code path. It also provides visibility into reviewer workload, allowing authors to avoid overloading already-busy reviewers. This removes the cognitive burden of knowing who to ask and reduces the tendency to pick whoever is most familiar regardless of workload.
What is the significance of Google’s OWNERS file system in Critique’s workflow?
?
OWNERS files declare which engineers are responsible for specific code paths. Critique uses these declarations to: (1) suggest appropriate reviewers when a CL touches code, (2) determine which reviewers can grant owner approval (one of the three required approval gates), and (3) surface ownership context in the review UI. Owner approval is distinct from LGTM — it attests that the change is appropriate for that codebase, not just that it is technically correct.
How does Critique handle large CLs that touch many files?
?
Critique provides: file tree navigation to move through changed files systematically; filtering by comment state (e.g., show only files with unresolved comments, or files not yet reviewed); and per-file review state marking so reviewers can track which files they have finished. These mechanisms allow reviewers to work through large CLs without losing their place, making thorough review of large changes practical rather than overwhelming.
What is the “who has the ball” principle in Critique’s notification design?
?
The most important notification is the one that signals it is your turn to act. Critique’s notification model is built around action-required transitions: the author is notified when a reviewer posts feedback or changes approval status; reviewers are notified when the author has updated the CL and re-requested review. Notifications that do not indicate a required action (e.g., “reviewer has been reading your CL”) are suppressed. The result is that every notification almost always means something actionable.
How does Critique’s build system integration support code review quality?
?
Critique shows build and test status directly on the CL — whether the change compiles against the current repository, whether existing tests pass, and whether new test failures are introduced. This information is visible to both author and reviewer. An author who sends a CL with failing builds or tests is making a visible mistake; the tool does not block it but makes the state transparent. Reviewers can confidently assess a CL’s readiness without running the build themselves.
What does it mean that a code review tool is “never neutral” with respect to behavior?
?
Every design decision in a code review tool — what information to surface, how notifications work, what approval state looks like, where analysis findings appear — encodes opinions about what good code review looks like. A tool that buries analysis findings in a separate tab trains engineers to ignore them. A tool that sends one notification per comment creates interruption-driven review habits. Critique’s design embodies specific values: analysis should be inline, state should be explicit, notifications should be high-signal. The tool actively shapes engineering behavior, for better or worse.
What is the “comment resolution” workflow in Critique?
?
After a reviewer posts comments, the author responds by marking each comment as either Done (the change was made as requested) or Acknowledged (the comment was noted but no change was made, with explanation). This closing-the-loop mechanism ensures every reviewer comment receives an explicit response — comments do not silently accumulate or get implicitly ignored. The review record shows whether each concern was addressed, creating a complete and auditable record of the review conversation.
What is the presubmit check stage in Critique’s workflow, and why does it run twice?
?
Presubmit checks run at two points: (1) at CL creation, giving the author immediate feedback on compilation, test status, and analysis findings before reviewers are engaged; and (2) immediately before submission, as a final gate to catch cases where other changes submitted to the repository in the interim have broken checks that previously passed. Running twice ensures that both the initial review and the final commit are against a clean state.
What is the significance of Tricorder’s “dismiss with feedback” mechanism?
?
When a reviewer or author dismisses a Tricorder finding as not useful or a false positive, that feedback is returned to the analyzer as a training signal. Over time, analyzers that generate high rates of dismissed findings are identified as low-quality and receive less prominence or are revised. This feedback loop allows the static analysis platform to self-improve based on real-world reviewer response, progressively reducing the false-positive rate for the findings that appear in Critique.
What are the key properties of an effective code review tool according to the chapter?
?
Tight integration with automated analysis (findings inline, not in separate reports); low-noise, high-signal notifications (only action-required events notify); explicit and visible review state (approval status, whose turn it is); context-preserving diffing (enough surrounding code to understand changes); inline commenting with draft support (batch feedback per review round); post-commit record archiving (CLs as permanent knowledge artifacts); and reviewer selection tooling (ownership-based suggestions, workload visibility).
Total Cards: 21
Review Time: ~22 minutes
Priority: MEDIUM
Last Updated: 2026-06-02