Conversation
…hanges Codifies the tolerant → release → strict discipline from the mobile compat audit: required-field additions and fallback removals get three deployments (tolerant backend, client release, strict flip), additive changes stay deploy-safe, and semantic tightening needs its backfills live first. Worked examples: teams header retirement (#2450/#2458), tasks payload org (#2457/#2459). Pointer added from graphql_errors.md's Related files.
Contributor
Reviewer's GuideAdds documentation for safely rolling out mobile-visible GraphQL schema changes across independently timed backend and app releases, centered on a tolerant → release → strict deployment sequence, and links that guidance from the GraphQL error reference. Sequence diagram for staged mobile API contract rolloutsequenceDiagram
participant Backend
participant MobileApp
participant Users
Note over Backend,MobileApp: Step 1: tolerant backend
Backend->>Backend: Accept new and legacy inputs
Users->>MobileApp: Open released app
MobileApp->>Backend: Send legacy input
Backend-->>MobileApp: Preserve legacy behavior
Note over Backend,MobileApp: Step 2: client release
MobileApp->>Backend: Send new input
Backend-->>MobileApp: Execute new path
Note over Backend,MobileApp: Step 3: strict flip after adoption
Backend->>Backend: Require new input or remove fallback
MobileApp->>Backend: Send new input
Backend-->>MobileApp: Execute strict contract
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="apps/betterangels-backend/docs/api-compatibility.md" line_range="18" />
<code_context>
+ - Old client → new backend: `Field 'organizationId' of required type 'ID!' was not provided`.
+ - So neither side can ship first alone — a **tolerant middle phase is mandatory** (below).
+2. **Never remove a field, mutation, or fallback** until clients using it are gone. Check usage first: `git grep <symbol> origin/main -- libs/expo`.
+3. **Additive changes are deploy-safe**: new fields, new *optional* inputs/arguments, new queries/mutations, and server-side directive swaps (`@hasPerm` etc. — clients never send directives). No staging needed.
+4. **Semantic tightening needs its data story**: if enforcement starts denying callers who previously succeeded, the conversion (grant/role backfills — e.g. the `post_migrate` grant backfills in `accounts/apps.py`) must be live **before** the enforcement ships.
+
</code_context>
<issue_to_address>
**issue:** The claim that new optional inputs/arguments and new queries/mutations are deploy-safe with no staging is false for clients that start using them before the backend is deployed: an old backend rejects the unknown input field or operation during GraphQL validation, so the mobile request fails.
**Triggers:** When a mobile client release reaches users before the backend containing the new optional input, argument, query, or mutation is live.
**Suggested fix:** Limit the no-staging rule to additive backend output fields, or state that the backend must be deployed before clients begin sending or calling newly added inputs, arguments, queries, or mutations.
```suggestion
3. **Additive backend output fields are deploy-safe**: server-side directive swaps (`@hasPerm` etc. — clients never send directives). For new *optional* inputs/arguments and new queries/mutations, deploy the backend before clients begin sending or calling them; no staging needed.
```
</issue_to_address>
### Comment 2
<location path="apps/betterangels-backend/docs/api-compatibility.md" line_range="40" />
<code_context>
+## Examples
+
+- **Teams — header retirement (DEV-2566).** `TeamFilter.organizationId` already existed and the backend already prefers the filter, falling back to the `X-Organization-ID` header — so the FE change alone was safe to ship **first** (#2458). The header/middleware retirement (#2450) and the interceptor removal (#2452) wait for the FE build's adoption.
+- **Tasks — payload org (RFC 0003 slice 1, DEV-2561).** `createTask` grew `organizationId`. Tolerant backend + mobile wiring (#2457, with `task_create_legacy` preserving the pre-cutover path), strict flip (#2459) gated on the app build being live.
+
+## What non-holders see
</code_context>
<issue_to_address>
**issue:** The worked Tasks example refers to a `task_create_legacy` compatibility path that does not exist anywhere in the repository, so readers cannot locate or verify the purported tolerant backend implementation and may copy an invalid symbol into rollout plans.
**Triggers:** When an engineer follows the Tasks example to inspect the tolerant implementation.
**Suggested fix:** Replace `task_create_legacy` with the actual implementation or link the specific PR/branch where that symbol exists; otherwise describe the legacy path without presenting it as a repository symbol.
```suggestion
- **Tasks — payload org (RFC 0003 slice 1, DEV-2561).** `createTask` grew `organizationId`. Tolerant backend + mobile wiring (#2457, with the legacy behavior preserved for pre-cutover clients), strict flip (#2459) gated on the app build being live.
```
</issue_to_address>| - Old client → new backend: `Field 'organizationId' of required type 'ID!' was not provided`. | ||
| - So neither side can ship first alone — a **tolerant middle phase is mandatory** (below). | ||
| 2. **Never remove a field, mutation, or fallback** until clients using it are gone. Check usage first: `git grep <symbol> origin/main -- libs/expo`. | ||
| 3. **Additive changes are deploy-safe**: new fields, new *optional* inputs/arguments, new queries/mutations, and server-side directive swaps (`@hasPerm` etc. — clients never send directives). No staging needed. |
Contributor
There was a problem hiding this comment.
issue: The claim that new optional inputs/arguments and new queries/mutations are deploy-safe with no staging is false for clients that start using them before the backend is deployed: an old backend rejects the unknown input field or operation during GraphQL validation, so the mobile request fails.
Triggers: When a mobile client release reaches users before the backend containing the new optional input, argument, query, or mutation is live.
Suggested fix: Limit the no-staging rule to additive backend output fields, or state that the backend must be deployed before clients begin sending or calling newly added inputs, arguments, queries, or mutations.
Suggested change
| 3. **Additive changes are deploy-safe**: new fields, new *optional* inputs/arguments, new queries/mutations, and server-side directive swaps (`@hasPerm` etc. — clients never send directives). No staging needed. | |
| 3. **Additive backend output fields are deploy-safe**: server-side directive swaps (`@hasPerm` etc. — clients never send directives). For new *optional* inputs/arguments and new queries/mutations, deploy the backend before clients begin sending or calling them; no staging needed. |
| ## Examples | ||
|
|
||
| - **Teams — header retirement (DEV-2566).** `TeamFilter.organizationId` already existed and the backend already prefers the filter, falling back to the `X-Organization-ID` header — so the FE change alone was safe to ship **first** (#2458). The header/middleware retirement (#2450) and the interceptor removal (#2452) wait for the FE build's adoption. | ||
| - **Tasks — payload org (RFC 0003 slice 1, DEV-2561).** `createTask` grew `organizationId`. Tolerant backend + mobile wiring (#2457, with `task_create_legacy` preserving the pre-cutover path), strict flip (#2459) gated on the app build being live. |
Contributor
There was a problem hiding this comment.
issue: The worked Tasks example refers to a task_create_legacy compatibility path that does not exist anywhere in the repository, so readers cannot locate or verify the purported tolerant backend implementation and may copy an invalid symbol into rollout plans.
Triggers: When an engineer follows the Tasks example to inspect the tolerant implementation.
Suggested fix: Replace task_create_legacy with the actual implementation or link the specific PR/branch where that symbol exists; otherwise describe the legacy path without presenting it as a repository symbol.
Suggested change
| - **Tasks — payload org (RFC 0003 slice 1, DEV-2561).** `createTask` grew `organizationId`. Tolerant backend + mobile wiring (#2457, with `task_create_legacy` preserving the pre-cutover path), strict flip (#2459) gated on the app build being live. | |
| - **Tasks — payload org (RFC 0003 slice 1, DEV-2561).** `createTask` grew `organizationId`. Tolerant backend + mobile wiring (#2457, with the legacy behavior preserved for pre-cutover clients), strict flip (#2459) gated on the app build being live. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docs-only. Codifies the rollout discipline the mobile compat audit produced (teams #2450/#2458, tasks #2457/#2459): released mobile binaries + instant backend deploys mean contract moves must be staged tolerant → release → strict, never in one step.
apps/betterangels-backend/docs/api-compatibility.md: the asymmetry, the four rules (no one-step required fields, no fallback removal before adoption, additive changes are safe, semantic tightening needs its backfills), the three-deployment recipe with the DO-NOT-LAND checklist convention, and worked examples from the two live cutovers.graphql_errors.md's Related files (denial shapes stay there; this doc is about when contracts move).Summary by Sourcery
Document the tolerant → release → strict rollout process for safely evolving API contracts used by released mobile clients.
New Features:
Enhancements:
Documentation: