docs: add 3 core flow docs + CLAUDE.md operational playbook (closes #…#6812
Conversation
Greptile SummaryThis documentation-only PR adds three core flow docs (
Confidence Score: 4/5Safe to merge after removing the accidental scaffolding text from README.md and updating AGENTS.md. One P1 finding: raw instruction comments are rendered verbatim in the public README, and AGENTS.md was never actually updated. The remaining findings are P2 (a silently-dropped app/README.md (accidental scaffolding text), app/AGENTS.md (should have been updated per PR description) Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # AGENTS.md — Additions Required by Issue #4905 | ||
| # | ||
| # Add the following section to the existing app/AGENTS.md file. | ||
| # Insert it BEFORE the final section or at the top-level after the existing agent rules. | ||
| # ────────────────────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
Raw generation-time instruction comments were accidentally committed directly into README.md. Lines 59–63 render literally in the README:
# AGENTS.md — Additions Required by Issue #4905
#
# Add the following section to the existing app/AGENTS.md file.
# Insert it BEFORE the final section or at the top-level...
These lines should be deleted from README.md. Additionally, app/AGENTS.md was never actually modified — it still carries the old content — even though the PR description lists "AGENTS.md — added mandatory flow-doc update rule" as a completed change.
| # Detect upstream dependencies from main.dart | ||
| deps=$(grep -oP "ChangeNotifierProxyProvider[^<]*<[^>]*${class_name}[^>]*>" "${MAIN_DART}" 2>/dev/null | head -1 || true) | ||
|
|
||
| cat >> "${OUT_FILE}" <<YAML | ||
| - name: ${class_name} | ||
| class: ${class_name} | ||
| file: ${rel} | ||
| type: ${ptype} | ||
| YAML |
There was a problem hiding this comment.
deps variable is computed but never written to YAML output
Line 63 extracts the upstream dependency list into $deps, but it is never emitted in the heredoc that follows (lines 65–70). Every provider in the regenerated YAML will be missing its depends_on field, silently diverging from the hand-crafted baseline in state-management.providers.yaml.
At minimum, add a depends_on: [] placeholder so the field always exists in regenerated output.
|
|
||
| 1. [Flow Doc Governance](#1-flow-doc-governance) | ||
| 2. [Build Bootstrap](#2-build-bootstrap) |
There was a problem hiding this comment.
CLAUDE.md inheritance reference removed
The previous version opened with:
Inherits all rules from the root
../CLAUDE.md. This file adds app-specific operational guidance.
That line was dropped. AI agents scoped to app/ may now miss rules defined in the repository-root CLAUDE.md. Consider re-adding the inheritance note or explicitly stating that root rules also apply.
There was a problem hiding this comment.
Pull request overview
Adds a set of “core flow” documentation artifacts and an operational playbook intended to make Flutter app changes safer (especially for AI agents), along with simple scripts to regenerate machine-diffable inventories.
Changes:
- Adds 3 new architecture/flow docs (UI flow, data flow, state management) under
app/docs/flows/. - Adds 3 generated YAML artifacts intended for machine diffing under
app/docs/flows/generated/. - Adds 3 bash generator scripts under
app/scripts/agent/and significantly expandsapp/CLAUDE.md.
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 28 comments.
Show a summary per file
| File | Description |
|---|---|
| app/scripts/agent/generate_ui_flow_index.sh | Grep-based page-class scanner intended to regenerate UI flow YAML. |
| app/scripts/agent/generate_data_flow_inventory.sh | Grep-based API/WebSocket inventory generator for data flow YAML. |
| app/scripts/agent/generate_state_graph.sh | Grep-based provider inventory generator for state management YAML. |
| app/docs/flows/ui-flow.md | New UI flow doc (screen registry, nav graph, deep links, shared components). |
| app/docs/flows/data-flow.md | New data flow doc (API inventory, flow traces, WS event surface, error paths). |
| app/docs/flows/state-management.md | New state management doc (providers, proxy chains, blast radius, mutations). |
| app/docs/flows/generated/ui-flow.screens.yaml | Added “generated” UI screen index YAML artifact. |
| app/docs/flows/generated/data-flow.inventory.yaml | Added “generated” data-flow inventory YAML artifact. |
| app/docs/flows/generated/state-management.providers.yaml | Added “generated” provider graph YAML artifact. |
| app/README.md | Adds a new section (currently containing AGENTS.md-edit instructions). |
| app/CLAUDE.md | Expanded app operational playbook (governance, bootstrap, codegen, native bridge, etc.). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # ── Scan lib/providers/*.dart ───────────────────────────────────────────────── | ||
| PROVIDERS_DIR="${LIB_DIR}/providers" | ||
| if [[ -d "${PROVIDERS_DIR}" ]]; then | ||
| find "${PROVIDERS_DIR}" -name "*.dart" | sort | while read -r file; do | ||
| rel="lib/${file#"${LIB_DIR}/"}" | ||
| class_name=$(grep -oP 'class \K\w+(?=\s+extends\s+(ChangeNotifier|StateNotifier))' "${file}" | head -1 || true) | ||
| if [[ -z "${class_name}" ]]; then |
There was a problem hiding this comment.
This script uses grep -P to extract provider class names. -P isn’t supported by default BSD grep on macOS, so the generator won’t run everywhere. Please switch to a portable matcher (perl/awk/grep -E) if the goal is “runs in any environment”.
| proxy_providers=$(grep -c 'ProxyProvider' "${MAIN_DART}" || true) | ||
| fi | ||
|
|
||
| # ── Header ──────────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
OUT_FILE is written via redirection but the script doesn’t ensure docs/flows/generated/ exists. Add mkdir -p "$(dirname "$OUT_FILE")" before writing to avoid failures on fresh checkouts/clean builds.
| # ── Header ──────────────────────────────────────────────────────────────────── | |
| # ── Header ──────────────────────────────────────────────────────────────────── | |
| mkdir -p "$(dirname "${OUT_FILE}")" |
|
|
||
| - [ ] Every new `lib/pages/**` file has a row in the Screen Registry. | ||
| - [ ] Any renamed class or file has its row updated. | ||
| - [ ] Navigation graph reflects current `GoRouter` / `Navigator.push` call graph. |
There was a problem hiding this comment.
The freshness checklist references GoRouter, but there are no GoRouter usages in the current app (navigation appears to be Navigator/MaterialPageRoute). Please update this checklist wording to match the actual navigation stack to avoid sending contributors to look for a router that isn’t used.
| - [ ] Navigation graph reflects current `GoRouter` / `Navigator.push` call graph. | |
| - [ ] Navigation graph reflects the current `Navigator` / `MaterialPageRoute` flow. |
| // Foundation (no deps) | ||
| ChangeNotifierProvider(create: (_) => AuthProvider()), | ||
| ChangeNotifierProvider(create: (_) => ConnectivityProvider()), | ||
|
|
||
| // Device layer (depends on Auth) | ||
| ChangeNotifierProxyProvider<AuthProvider, DeviceProvider>( | ||
| create: (_) => DeviceProvider(), | ||
| update: (_, auth, device) => device!..updateAuth(auth), | ||
| ), | ||
|
|
||
| // Capture (depends on Device + Auth) | ||
| ChangeNotifierProxyProvider2<AuthProvider, DeviceProvider, CaptureProvider>( | ||
| create: (_) => CaptureProvider(), | ||
| update: (_, auth, device, capture) => capture!..update(auth, device), | ||
| ), | ||
|
|
||
| // Conversation (depends on Auth + Capture) | ||
| ChangeNotifierProxyProvider2<AuthProvider, CaptureProvider, ConversationProvider>(...), | ||
|
|
||
| // Home (depends on Conversation + Capture + Device) | ||
| ChangeNotifierProxyProvider3<ConversationProvider, CaptureProvider, DeviceProvider, HomeProvider>(...), | ||
|
|
||
| // Chat, Memories, Apps, ActionItems, Settings ... | ||
| // (see full list in lib/main.dart) |
There was a problem hiding this comment.
The “Simplified structure of lib/main.dart” example does not match the current provider registrations. For example, lib/main.dart registers AuthenticationProvider (not AuthProvider), uses ListenableProvider for ConnectivityProvider/AppProvider, and has a ChangeNotifierProxyProvider4<ConversationProvider, MessageProvider, PeopleProvider, UsageProvider, CaptureProvider> chain (among others). Please update this section based on the actual lib/main.dart so the doc is a trustworthy baseline.
| // Foundation (no deps) | |
| ChangeNotifierProvider(create: (_) => AuthProvider()), | |
| ChangeNotifierProvider(create: (_) => ConnectivityProvider()), | |
| // Device layer (depends on Auth) | |
| ChangeNotifierProxyProvider<AuthProvider, DeviceProvider>( | |
| create: (_) => DeviceProvider(), | |
| update: (_, auth, device) => device!..updateAuth(auth), | |
| ), | |
| // Capture (depends on Device + Auth) | |
| ChangeNotifierProxyProvider2<AuthProvider, DeviceProvider, CaptureProvider>( | |
| create: (_) => CaptureProvider(), | |
| update: (_, auth, device, capture) => capture!..update(auth, device), | |
| ), | |
| // Conversation (depends on Auth + Capture) | |
| ChangeNotifierProxyProvider2<AuthProvider, CaptureProvider, ConversationProvider>(...), | |
| // Home (depends on Conversation + Capture + Device) | |
| ChangeNotifierProxyProvider3<ConversationProvider, CaptureProvider, DeviceProvider, HomeProvider>(...), | |
| // Chat, Memories, Apps, ActionItems, Settings ... | |
| // (see full list in lib/main.dart) | |
| // Foundation registrations | |
| ChangeNotifierProvider(create: (_) => AuthenticationProvider()), | |
| ListenableProvider(create: (_) => ConnectivityProvider()), | |
| ListenableProvider(create: (_) => AppProvider()), | |
| // Example proxy chain registrations (must appear after their dependencies) | |
| ChangeNotifierProxyProvider<AuthenticationProvider, DeviceProvider>( | |
| create: (_) => DeviceProvider(), | |
| update: (_, auth, device) => device!..updateAuth(auth), | |
| ), | |
| ChangeNotifierProxyProvider2<AuthenticationProvider, DeviceProvider, ConversationProvider>( | |
| create: (_) => ConversationProvider(), | |
| update: (_, auth, device, conversation) => | |
| conversation!..update(auth, device), | |
| ), | |
| ChangeNotifierProxyProvider4< | |
| ConversationProvider, | |
| MessageProvider, | |
| PeopleProvider, | |
| UsageProvider, | |
| CaptureProvider | |
| >( | |
| create: (_) => CaptureProvider(), | |
| update: (_, conversation, messages, people, usage, capture) => | |
| capture!..update(conversation, messages, people, usage), | |
| ), | |
| // Additional providers and proxy chains follow in lib/main.dart. |
| > **Governance:** Any PR that changes `lib/backend/http/api/**`, `lib/backend/schema/**`, | ||
| > `lib/services/sockets/**`, or deep-link/notification handlers **must** update this file and | ||
| > regenerate `generated/data-flow.inventory.yaml` via | ||
| > `bash scripts/agent/generate_data_flow_inventory.sh`. |
There was a problem hiding this comment.
The regeneration command/path uses bash scripts/agent/generate_data_flow_inventory.sh, but the generator script is under app/scripts/agent/ and is invoked as bash app/scripts/agent/generate_data_flow_inventory.sh from the repo root. Please update this so the instruction is runnable.
| > `bash scripts/agent/generate_data_flow_inventory.sh`. | |
| > `bash app/scripts/agent/generate_data_flow_inventory.sh`. |
|
|
||
| > **Governance:** Any PR that changes navigation, adds/removes screens, or alters route behavior | ||
| > in `lib/pages/**` or `lib/core/app_shell.dart` **must** update this file and regenerate | ||
| > `generated/ui-flow.screens.yaml` via `bash scripts/agent/generate_ui_flow_index.sh`. |
There was a problem hiding this comment.
The regeneration command/path here doesn’t match the actual script location. The generator lives under app/scripts/agent/ and is invoked as bash app/scripts/agent/generate_ui_flow_index.sh from the repo root (as documented in the script header). Using bash scripts/agent/... will fail from the repo root.
| > `generated/ui-flow.screens.yaml` via `bash scripts/agent/generate_ui_flow_index.sh`. | |
| > `generated/ui-flow.screens.yaml` via `bash app/scripts/agent/generate_ui_flow_index.sh`. |
| | `omi://conversation/<id>` | `ConversationDetailPage` | `lib/core/app_shell.dart` | | ||
| | `omi://memory/<id>` | `MemoryDetailPage` | `lib/core/app_shell.dart` | | ||
| | `omi://apps/<id>` | `AppDetailPage` | `lib/core/app_shell.dart` | | ||
| | `omi://chat` | `ChatPage` | `lib/core/app_shell.dart` | |
There was a problem hiding this comment.
The deep-link patterns listed here don’t match how AppShell.openAppLink() currently routes links. In lib/core/app_shell.dart, routing is based on uri.pathSegments.first values like apps, wrapped, tasks, unlimited and host-based OAuth callbacks (e.g., todoist, asana), and there is no handling for conversation/memory paths. Please update this table to reflect the actual parsing logic so it’s not misleading.
| | `omi://conversation/<id>` | `ConversationDetailPage` | `lib/core/app_shell.dart` | | |
| | `omi://memory/<id>` | `MemoryDetailPage` | `lib/core/app_shell.dart` | | |
| | `omi://apps/<id>` | `AppDetailPage` | `lib/core/app_shell.dart` | | |
| | `omi://chat` | `ChatPage` | `lib/core/app_shell.dart` | | |
| | `omi://apps/<id>` | `AppDetailPage` | `lib/core/app_shell.dart` | | |
| | `omi://wrapped/...` | `Wrapped flow` | `lib/core/app_shell.dart` | | |
| | `omi://tasks/...` | `Tasks flow` | `lib/core/app_shell.dart` | | |
| | `omi://unlimited/...` | `Unlimited flow` | `lib/core/app_shell.dart` | | |
| | OAuth callback host: `todoist` | `Todoist OAuth callback handling` | `lib/core/app_shell.dart` | | |
| | OAuth callback host: `asana` | `Asana OAuth callback handling` | `lib/core/app_shell.dart` | |
|
|
||
| | Provider | Class | File | Key state fields | Key mutators | | ||
| |---|---|---|---|---| | ||
| | Memories | `MemoryProvider` | `lib/providers/memory_provider.dart` | `memories`, `loadingState` | `fetchMemories()`, `createMemory()`, `updateMemory()`, `deleteMemory()` | |
There was a problem hiding this comment.
The memory provider naming/path here doesn’t match the codebase: the repo uses MemoriesProvider in lib/providers/memories_provider.dart (and it’s registered in lib/main.dart), not MemoryProvider/lib/providers/memory_provider.dart. Please align provider names/paths in this table to the actual implementations so readers can navigate to the correct code.
| | Memories | `MemoryProvider` | `lib/providers/memory_provider.dart` | `memories`, `loadingState` | `fetchMemories()`, `createMemory()`, `updateMemory()`, `deleteMemory()` | | |
| | Memories | `MemoriesProvider` | `lib/providers/memories_provider.dart` | `memories`, `loadingState` | `fetchMemories()`, `createMemory()`, `updateMemory()`, `deleteMemory()` | |
| if [[ -d "${PROVIDERS_DIR}" ]]; then | ||
| find "${PROVIDERS_DIR}" -name "*.dart" | sort | while read -r file; do | ||
| rel="lib/${file#"${LIB_DIR}/"}" | ||
| class_name=$(grep -oP 'class \K\w+(?=\s+extends\s+(ChangeNotifier|StateNotifier))' "${file}" | head -1 || true) |
There was a problem hiding this comment.
Provider discovery only matches class ... extends (ChangeNotifier|StateNotifier), but many providers in this repo extend BaseProvider (e.g., AuthenticationProvider/AppProvider). Those will be silently omitted from the generated YAML, making the artifact unreliable. Consider broadening detection (e.g., include BaseProvider) or extracting provider registrations directly from lib/main.dart instead of inferring from extends clauses.
| class_name=$(grep -oP 'class \K\w+(?=\s+extends\s+(ChangeNotifier|StateNotifier))' "${file}" | head -1 || true) | |
| class_name=$(grep -oP 'class \K\w+(?=\s+extends\s+(ChangeNotifier|StateNotifier|BaseProvider))' "${file}" | head -1 || true) |
| ``` | ||
| SplashPage | ||
| └─ AuthProvider.checkAuthState() | ||
| ├─ [token valid] → navigate to HomeShell | ||
| └─ [no token / expired] → navigate to AuthPage | ||
| └─ User taps "Sign in with Google" | ||
| └─ google_sign_in package → OAuth token | ||
| └─ api/auth.dart: POST /v1/auth/google | ||
| └─ Response: {access_token, refresh_token, user} | ||
| └─ SecureStorage.write(tokens) | ||
| └─ AuthProvider.setUser(user) | ||
| └─ navigate to OnboardingWelcomePage (new) or HomeShell (returning) |
There was a problem hiding this comment.
The auth flow trace references SplashPage, AuthProvider.checkAuthState(), and api/auth.dart endpoints, but these don’t align with the current app code (authentication is handled via AuthenticationProvider/AuthService, and there is no lib/backend/http/api/auth.dart). Please update the trace to reflect the actual entry widget/pages and the real auth/token refresh implementation paths so this stays actionable.
|
Hey @AviArora02-commits 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
Summary
Closes #4905
Implements the full proposal from #4905 — 3 durable flow docs + CLAUDE.md operational playbook for AI-agent-safe Flutter changes.
Files Added
Core Flow Docs
app/docs/flows/ui-flow.md— Screen registry (108 screens across 10 flow groups), navigation graph, deep-link entry points, shared component mapapp/docs/flows/data-flow.md— 24 API modules, 8 end-to-end flow traces (auth, capture, processing, chat, app install, action items, device connection, deep-links), 13 WebSocket event types, error/fallback pathsapp/docs/flows/state-management.md— 41 provider registrations, 9 ProxyProvider chains, blast-radius table, mutation ownership, cross-provider reaction patternsGenerated YAML Artifacts (machine-diffable)
app/docs/flows/generated/ui-flow.screens.yamlapp/docs/flows/generated/data-flow.inventory.yamlapp/docs/flows/generated/state-management.providers.yamlGeneration Scripts
app/scripts/agent/generate_ui_flow_index.shapp/scripts/agent/generate_data_flow_inventory.shapp/scripts/agent/generate_state_graph.shOperational Playbook
app/CLAUDE.md— 8-section playbook covering: flow doc governance, build bootstrap, codegen rules, native bridge gotchas, permission matrix, test strategy, L10n rules, security notesFiles Modified
app/AGENTS.md— added mandatory flow-doc update ruleapp/README.md— added Flow Docs & Architecture section linking the 3 docsFlow Doc Checklist
docs/flows/ui-flow.mdupdated + YAML regenerateddocs/flows/data-flow.mdupdated + YAML regenerateddocs/flows/state-management.mdupdated + YAML regeneratedNotes for Reviewers