← CatZep/Engineering Guides/Management & Leadership/Code Review
The most expensive review miss isn't a bug — it's a breaking change that ships to a hundred consumers you didn't know about.
Student edition · understand, apply, verify
Review binary, schema, and semantic compatibility across old/new readers and writers.
What you should be able to do: Identify field meaning and migration/rollback requirements, not just whether a change is optional.
A context-dependent practice guide, not a universal policy or verified personal history. Examples and numbers are illustrative. Use genuine, authorized evidence; personnel/privacy/legal decisions follow applicable organization policies and qualified review.
Read these definitions first, or use the linked terms as you go. The examples and limitations are local to this guide; companion pages are optional.
Backward compatibility lets new readers read old data; forward lets old readers read new data. Field identity/default rules differ by format.
Example. Milliseconds changed to seconds under an unchanged integer field is a breaking semantic change.
Boundary. Optional is not universally safe. Registry checks are configured structural rules, not business semantics; schema-ID headers are format/vendor conventions, not Kafka itself.
Change a schema in stages that allow old and new software to coexist: add a compatible representation, deploy readers/writers, backfill and reconcile, switch authority, then remove old fields only after the rollback horizon.
Example. Rename full_name by adding display_name first. Both app versions remain valid until backfill, dual-write consistency, and rollback checks are satisfied.
Boundary. Two application writes are not automatically atomic. Prefer one transaction/change-log authority and detect divergence. Dropping the old field too soon makes code rollback unsafe.
Ownership names who drives follow-through; accountability names who answers for outcomes. Decision rights specify who chooses/approves and who is consulted.
Example. A migration has a coordinating owner, service owners, and named release authority.
Boundary. DRI/RACI/DACI are local conventions, not guarantees. Role labels do not replace authority, capacity, and follow-through.
A precondition states required inputs/state before an operation. A postcondition states the result promised when those requirements hold and execution terminates normally. Together they make requirements testable.
Example. For sum_first(b,n), require integer 0≤n≤len(b) and numeric elements; promise the sum of b[0:n].
Boundary. State exceptions, mutation, numeric domains, and empty cases. A mathematical array range does not automatically validate a runtime input.
Outputs are produced things; outcomes are changes for users/business/team. Useful metrics state definition, denominator, population, period, and attribution limits.
Example. 'Shipped a tool' is output. 'Median onboarding fell ten to six days in this cohort' is an outcome with sample/context caveats.
Boundary. Before/after does not prove sole causation. Avoid gaming, selection bias, hidden costs, and unlike-team comparisons.
In a large system, interfaces are contracts, and a diff that changes a contract can break producers and consumers that the author never sees. This question tests whether you review schema and API changes with the whole ecosystem in mind: who reads this field, who writes it, what happens during the window when old and new code both run. For an events-infra engineer this is bread and butter — Thrift/protobuf/Avro schemas flow through pipelines and land in tables that dozens of teams query, so a careless field change is a silent data-corruption or outage event, not a local bug.
Whether you think in terms of rollout windows and independent deployment. The core insight is that producers and consumers are never upgraded atomically, so every change must be safe when old and new versions coexist. The tell is that you reach for “additive and backward-compatible” by default, know the concrete wire-format rules that make a change safe or breaking, and always ask “who are the consumers?” before approving.
How to answerThe classic wire-format catch — a protobuf change that looks harmless and silently corrupts every consumer:
// WRONG: reused tag 2 and changed a type -- old readers misparse the bytes
message AdEvent {
string ad_id = 1;
int32 region_id = 2; // was: string campaign_id = 2; BREAKING
}
// RIGHT: reserve the old tag/name, add the new field with a fresh tag
message AdEvent {
reserved 2; // never recycle tag 2
reserved "campaign_id";
string ad_id = 1;
int32 region_id = 3; // additive: new tag, optional, defaulted
}This is an illustrative situation. The point is the reasoning and evidence, not copying the numbers or first-person story.
| Step | Evidence, decision, and expected result |
|---|---|
| Describe old meaning | duration is milliseconds and readers depend on that unit. |
| Keep the same integer field but change units | New code emits seconds. Structural checks pass, but old consumers silently compute wrong results. |
| Introduce an explicit new representation | Use a new field/version with documented units, defaults, reader/writer compatibility, and migration scope. |
| Preserve rollback/history | Backfill/reconcile under one authority, verify consumers and archived data, and retain the old meaning through the supported rollback horizon. |
No. Binary identity uses tags, but JSON/text mapping, reflection, APIs, and tooling can depend on names. Check every supported representation.
Download guide_labs.py. Python 3.10+, standard library only. In its directory, run:
python3 guide_labs.py compatibility
python3 guide_labs.py testThe lab checks old/new reader fixtures and demonstrates a units change that passes a shape-only check.
The program tests its stated finite contracts. It is an educational model, not an implementation of the named production product or a proof of unmodeled distributed faults.