Code Quality & Collaboration

4.1 Clean code and readability

Code is read far more often than it is written, and on a two-team codebase it’s usually read by someone other than the person who wrote it. Optimize for the next reader’s ability to understand intent quickly, not for the fewest keystrokes to make something work.

Names carry meaning. A variable, function, or component name should make a comment unnecessary. getActiveUsersInTenant needs no comment; getUsers2 or data does, and adding the comment is treating a symptom instead of fixing the name. See 3.3 for the specific naming conventions.
Functions and components do one thing. If you can’t summarize what a function does in one sentence without using “and,” it’s doing more than one thing and should be split. This applies equally to a backend service function and a frontend component: a component that fetches data, transforms it, and renders three unrelated pieces of UI is three components wearing one name.
Keep nesting and branching shallow. More than two or three levels of nested conditionals or callbacks is a signal to extract a function, use early returns, or invert a condition, not a signal to add more comments explaining the nesting.
Comments explain why, not what. Well-named code already says what it does. A comment earns its place only when it captures something the code can’t: a non-obvious constraint, a workaround for a specific bug, or a reason a simpler approach won’t work. A comment that restates the next line in English is noise and will drift out of sync with the code it describes.
Duplication is fine until it isn’t. Two or three similar lines of code are cheaper to read than a premature abstraction built to avoid them. Don’t extract a shared helper until a real third case shows up and the shared shape is actually stable, not just superficially similar (see 8.4 YAGNI and 3.4 on when to promote code to a shared package).
A function’s signature should not lie. If a function can throw, return null, or fail silently, that should be visible in its type signature or name (e.g. findUser returning User | null vs getUser that throws), not something a caller has to discover by reading the implementation.

Rule: if a reviewer has to ask “what does this do” or “why is this here” about a piece of code that isn’t inherently complex domain logic, that’s a readability defect in the code, not a gap in the reviewer’s understanding. Fix the code, don’t just answer the question in a PR comment and move on.

Inline and module documentation standard. The “why, not what” principle above governs individual comments; this is the standard for when documentation is required at all, and in what form.

Backend functions and API handlers that aren’t self-explanatory from their name and type signature get a TSDoc doc comment. A one-line summary of what the function does, @param for any parameter whose purpose or valid range isn’t obvious from its name and type, @returns for non-obvious return behavior, and @throws for anything that can throw, naming what triggers it. Skip a tag that would just restate the type signature, a @param userId on a parameter typed userId: string and named nothing else adds nothing the signature didn’t already say.
Frontend components that aren’t self-explanatory from their name and props type get a TSDoc doc comment, scoped to what a component’s doc comment actually needs to cover: a one-line summary of what the component renders and when to use it, a note on any non-obvious prop (documented on the prop itself in the props type, not repeated as a separate @param list), and any side effect or re-render behavior that isn’t visible from the component’s return value (a component that mutates external state, subscribes to an event, or triggers a re-render on something other than its own props or state).
Either way, a trivial exported function or component whose name and signature already say everything (a one-line getter, a straightforward re-export, a component that only renders its props into markup) does not need a doc comment, the same “does this comment earn its place” test from the bullet above applies to doc comments too.
A module doing something non-obvious at a glance gets a short file-level comment at the top: what the module is responsible for and why it exists as a separate module, not a restatement of its filename. A module whose purpose is already obvious from its name and its directory (a thin route handler, a single-component file) does not need one.
A non-obvious constraint gets a doc comment even if the function itself looks simple. If a function’s behavior depends on something the reader can’t see from the code itself, a specific host/environment rule, an ordering requirement relative to another module, a workaround for a bug in a dependency, that constraint is documented at the point it applies, not left for the next engineer to discover by breaking it.
A doc comment that only restates the function name in sentence form is noise, same as any other comment that doesn’t earn its place. /** Gets the user. */ above getUser() fails the same test as an inline comment that repeats the next line.

Rule: an exported function, component, or handler with non-obvious parameter behavior, a thrown-error path, or a constraint invisible from the code itself, and no doc comment explaining it, is a review flag. A reviewer who has to read the implementation to know how to safely call a function is hitting the same readability defect named above, just one layer up, at the API boundary of the module instead of inside it.

All documentation is written in English. Code comments, doc comments, commit messages, PR descriptions, README and other repository documentation, API documentation (including generated OpenAPI/Swagger output, see 6.3), and API response content intended for another system or a developer audience (error messages, field descriptions, log messages) are all written in English, with no exception for internal-only or “nobody external will see this” code. This document, this standard’s own web publication, and this rule apply to both teams equally.

English is the one language every engineer on both teams and every future hire is guaranteed to share. Mixed-language comments and documentation are unreadable to whoever doesn’t share the other language, and untranslatable by tooling that assumes one language across a codebase (linters, doc generators, AI coding agents reading the repository).
This rule is about developer-facing and system-facing text, not end-user-facing product copy. If a product surface has its own localization requirement, that’s a product decision made separately from this standard, not an exception to it.

Rule: a PR introducing a non-English code comment, doc comment, commit message, or piece of API documentation does not merge. This is enforced the same way as the rest of 4.3, a reviewer catching it in review is a backstop, not the primary enforcement mechanism.

No any. In TypeScript code, any is not an acceptable escape hatch. It silently disables the type checker for everything downstream of it, which defeats the type-safety criterion this stack was chosen against (see 2.1) and reintroduces exactly the class of bug static types exist to catch.

If the real shape is known, type it, even if that means writing out an interface for a third-party response.
If the shape is genuinely dynamic or not yet known, use unknown and narrow it before use (a type guard, a Zod parse, see 9.3), not any.
A generic function that needs to accept varying types should use a type parameter, not any.
// eslint-disable comments suppressing an any warning require a one-line justification in the comment and should be rare enough that one showing up in a PR prompts a reviewer to ask why, not to wave it through.

Rule: any is a lint error, not a warning, enforced in CI alongside the rest of 4.3. A PR introducing a new any does not merge without an explicit, reviewed justification in the PR description.

4.2 Pull request checklist

Every PR is reviewed against this checklist before merge, regardless of who’s reviewing or how small the change looks. A reviewer approving a PR that skips one of these items is as responsible for the gap as the author.

Loading and error states are handled. Any UI that fetches or mutates data shows a loading state and an error state, not just the happy path. A PR that only demonstrates the success case in its description is incomplete.
Mobile and accessibility are considered. The change works at mobile viewport widths (see 5.11) and meets the accessibility baseline (see 5.18), not just on the reviewer’s desktop browser at default zoom.
Tests are updated. New behavior has new tests; changed behavior has updated tests; a bug fix has a regression test proving the bug is fixed and stays fixed. See Section 13 for how much and where.
The PR description states the why, not just the what. The diff already shows what changed; the description should explain the reasoning a diff can’t show, so a future reader understands why the change was made this way.
The PR links its tracked issue. Reference the DevOps issue the work came from (e.g. Closes #123) so the Projects board updates automatically on merge and reviewers have the original task for context.
Scope matches the stated problem. A PR that also refactors unrelated code, renames unrelated variables, or reformats untouched files makes the actual change harder to review and harder to revert. Split unrelated work into a separate PR.

Rule: if a PR is missing one of these and there’s a good reason (e.g. tests are covered by an existing suite, mobile doesn’t apply to a backend-only change), the author states that reason in the PR description rather than the reviewer having to ask for it.

4.3 Linting and formatting

Linting and formatting rules exist to remove low-value review discussion (spacing, quote style, import order) so review time goes to correctness and design instead. The principle is tooling-agnostic: whatever tools are in use, they should be automated and non-negotiable at the point of commit or CI, not a matter of individual taste enforced by human review comments.

Formatting is automatic, not manual. Nobody should be leaving a review comment about whitespace or quote style; a formatter should have already made that discussion impossible.
Every repository exposes a standard format script (npm run format to write, npm run format:check to verify without writing), the same way every repository exposes lint. CI runs format:check, never format, a CI run must never write to the working tree, it fails and tells the author to run format locally instead.
Linting enforces correctness-adjacent rules, not just style: unused variables, unreachable code, missing dependency array entries, and similar classes of bug-shaped patterns.
CI is the source of truth. A local editor integration is a convenience, not the enforcement mechanism. If it’s not enforced in CI, it isn’t actually enforced.

Currently enforced: ESLint and Prettier, run in CI on every PR. A PR that fails lint or formatting checks does not merge, regardless of the severity of the individual violation.

4.4 Commit and branching convention

Convention: Conventional Commits, trunk-based development.

Trunk-based development is the explicit choice over GitFlow-style long- lived branches: with two teams sharing one standard, long-lived feature branches accumulate merge conflicts and drift from main in ways that are expensive to reconcile, and they hide integration problems until the merge instead of surfacing them early. Trunk-based development forces smaller, more frequent merges to main, which keeps integration continuous instead of a one-time event at the end of a feature.

Branches are short-lived. A feature branch should be merged within days, not weeks. If a feature is too large to land in that window, split it behind a feature flag (see 8.9) rather than keeping a long-lived branch alive.
Work is tracked in DevOps Projects. Tasks are assigned and tracked on the organization’s DevOps Projects board, so every piece of work in progress has a corresponding DevOps issue.
Branch names reference the issue number. A branch working on a tracked task includes that issue’s number, e.g. feat/123-refresh-token-handling or 123-refresh-token-handling. This lets anyone trace a branch back to the task that motivated it without having to ask, and lets DevOps auto-link the branch and its PR to the issue.
Commit messages follow Conventional Commits (type(scope): description, e.g. fix(auth): handle expired refresh token). This makes history scannable, supports automated changelog generation, and makes the intent of a commit legible without opening the diff.
Common types: feat, fix, refactor, test, docs, chore, perf. Use the type that matches intent, not the one that sounds most significant, a refactor mislabeled as feat misleads anyone scanning history later.
main is always deployable. Since deployment itself is out of scope for this document (see 1.2), this rule is about code health, not release process: nothing merges to main that isn’t in a working state, because trunk-based development means main is the shared integration point at all times, not just at release time.

Rule: a PR description or commit message that doesn’t state a clear type and scope is a sign the change itself may be doing more than one thing. Use that as a prompt to split the PR, not just to fix the message.

4.5 Code review practice

The checklist in 4.2 defines what a review checks. This subsection defines how review itself works between a frontend and a backend team sharing one standard, since a review process left implicit tends to default to whatever the loudest or most senior reviewer happens to do.

Review turnaround. A PR gets a first response, an approval, a requested change, or a clarifying question, within one business day of being opened. “First response” does not mean “fully reviewed,” a reviewer who can’t do a full review that day says so and gives a realistic estimate, rather than leaving the PR silently unacknowledged.

A PR open longer than two business days with no reviewer activity is the author’s signal to ping directly, not wait indefinitely assuming it’s been seen.
Turnaround expectations scale down, not up, for anything blocking another engineer or fixing a live issue: flag it as such in the PR description so a reviewer can prioritize it ahead of routine work.

Blocking vs. non-blocking feedback. A reviewer marks each piece of feedback as one or the other, explicitly, so an author isn’t left guessing whether a comment has to be resolved before merge:

Blocking: the PR does not merge until this is addressed, correctness issues, anything in the 4.2 checklist, anything this document states as a rule.
Non-blocking (a nit, a suggestion, a question): worth raising, not worth holding the PR for. Mark it as such (e.g. prefix with nit:) so the author can use judgment on whether to address it now or file a follow-up, rather than treating every comment as a blocker by default.

Review depth scales with change size and risk, not a fixed process applied uniformly. A small, low-risk change (a copy fix, a config value) warrants a fast, light review. A change touching auth, payment logic, a shared package (see 3.4), or a migration (see 6.6) warrants a closer read regardless of how small the diff looks, line count is not a reliable proxy for risk.

Disagreement resolution. Most review feedback resolves through discussion in the PR. When author and reviewer genuinely disagree on a non-objective call, something this document doesn’t already settle with a stated rule, the deciding order is:

Check whether this document already has a rule for it. If it does, the rule wins, that’s the point of writing rules down instead of litigating the same question every time it comes up.
If it doesn’t, the author and reviewer resolve it directly, in the PR thread or a short conversation, not a long asynchronous back-and-forth.
If they still disagree after that, the engineering lead makes the call, the same authority that signs off on changes to this document itself (see 1.3).
If the disagreement reveals a real gap in this document, that author or reviewer proposes closing it using the process in 1.3, so the next engineer who hits the same question has a rule to point to instead of relitigating it.

Rule: a PR is never merged over a reviewer’s unresolved blocking objection, and a reviewer never blocks a PR indefinitely on a non-blocking preference. Both are collaboration failures the process above exists to prevent, not acceptable outcomes to work around.

Code ownership across teams. This document exists specifically because two teams are held to one standard, review expectations follow from that, not from which team wrote the code:

Anyone on either team can review any PR whose content they can reasonably evaluate against this document, a frontend engineer reviewing a well-contained backend change (or vice versa) is encouraged, not just tolerated, it’s part of how the two teams stay aligned on one standard instead of drifting into two.
A change to a shared package (see 3.4) or to something both teams depend on gets review from at least one engineer on each affected team, since a shared dependency change that only one team evaluated is exactly how the two teams end up depending on incompatible assumptions about the same code.
Domain-specific correctness (whether a UI change is visually/behaviorally right, whether a query’s business logic is correct) still benefits from a reviewer who knows that domain, cross- team review does not replace a domain-knowledgeable reviewer, it supplements the standards-and-structure review with someone who can evaluate the specific problem being solved.

Rule: “that’s not my team’s code” is not a reason to decline a review request. If you can evaluate the change against this document, you’re a valid reviewer for it, regardless of which team wrote it.