Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
219 lines (164 loc) · 9.39 KB

CONTRIBUTING.md

File metadata and controls

219 lines (164 loc) · 9.39 KB
Raw

Contributing to Recall

Thanks for looking. This file covers everything between git clone and a merged PR, including the handful of traps that will otherwise cost you an evening.

If something here is wrong or out of date, that's a bug — please open an issue.

Ground rules

Two constraints are non-negotiable. Everything else is negotiable, and you're welcome to argue with it in an issue.

1. server/ never learns anything about a user.

No database, no session, no accounts, no cache of message content, no logging of anything a user typed. It relays a request to an LLM provider and forgets. Every byte of user data lives in SQLite and MMKV on the device.

This isn't caution, it's the product: it's what makes "delete all my data" truthful, and it's the only reason the consent notice can say what it says. A PR that gives the server memory of a user needs a design discussion first, not a review.

2. The agent suggests; it does not act.

Nothing that sends, posts, deletes, or otherwise touches the outside world happens without an explicit tap. The autonomy model is Suggest → Confirm → Auto, per capability, defaulting to Suggest, and Auto is deliberately not in the MVP.

Both rules are stated in full in AGENT.md §2 and §7. Read that file before writing code — it also holds the architecture and the tech-stack list.

Setup

Prerequisites
Node 22.18 or newer (the server's tooling relies on native TypeScript type stripping)
JDK 17 or newer
Android Android Studio + SDK, and a device or emulator on API 24+
Device Real hardware strongly preferred — notification access, calendar and mic can't be meaningfully tested on an emulator

Expo Go will not work. MMKV, expo-sqlite, and the notification listener are native modules. You need a dev build, which means npx expo run:android, not npx expo start on its own.

git clone https://github.com/trh-ds/recall.git
cd recall
npm install
cp .env.example .env
npx expo run:android

npm install runs patch-package via postinstall. If a fresh clone won't build, that's the first thing to check — see the traps below.

The relay is a separate npm project. It has zero runtime dependencies on purpose, so npm install there only pulls TypeScript and @types/node:

cd server
npm install
cp .env.example .env.local     # one provider key is enough
npm test                       # node --test, offline
npm run typecheck
node --env-file=.env.local dev.mjs

dev.mjs is a ~25-line plain-Node runner. It exists so you don't need a Vercel account to work on the server, and it doubles as proof that the handlers aren't tied to Vercel.

You do not need any of this to work on most of the app. The daily briefing — the home screen, the largest feature — makes no LLM call at all. It derives everything from SQLite synchronously. No keys, no relay, no network.

Traps

These have all bitten someone already. They're also recorded in TODO.md as they're found.

npm run typecheck fails on a route that clearly exists. expo-router generates route types into .expo/types/router.d.ts. Until Metro has run once since you added a screen, typecheck doesn't know the route. Start the app once (npx expo start), let it regenerate, then re-run. Not a real error.

A native module dies on launch with no Metro output. Run adb logcat -b crash -d and then npx expo install --check. The usual cause is version skew between an Expo SDK module and what's in package.json — SDK 57 renumbered several modules, and an old pin crashes before JS starts (for example expo-sqlite@16.x throws NoClassDefFoundError: AnyTypeProvider).

A fresh clone won't build at all. react-native-android-notification-listener was last published in 2022, is peer-locked to React 18, and needs patches/react-native-android-notification-listener+5.0.1.patch to compile against AGP 8 at all. patch-package applies it on postinstall. If you deleted node_modules in an unusual way, re-run npm install.

Never sideload a release APK onto a Xiaomi. HyperOS rejects it as "security reinforcement" and the failure is opaque. Use the debug APK.

The entry point is index.ts, not expo-router/entry. The notification listener's headless task must be registered while the app is killed, so index.ts registers it and imports expo-router/entry first. Routing is unchanged. Don't let a scaffold tool "fix" main in package.json back.

A bare HTTP 404 from LLM failover usually means a retired model, not a bad key. Providers drop model IDs without warning — Groq retired llama-3.1-8b-instant mid-development, and the symptom looks exactly like an auth failure. GET /api/health shows which keys are configured; check the provider's /models list before assuming the key is dead.

Don't put a TextInput inside a Modal. On Android a Modal is its own window, so android:windowSoftInputMode="adjustResize" doesn't apply to it and the keyboard covers whatever you're typing into. Render the panel inline instead — the activity resizes and an absolutely-positioned card rides up with it. This is why the capture review card lives in the dock and only the mic overlay is a Modal (src/components/voice-capture.tsx).

Settle a permission before showing UI that assumes it. The OS permission prompt is a native window and will stack on top of your overlay, leaving the user with two dialogs and a mic that isn't recording. ensureMic() is idempotent — call it first, then show the listening state.

Windows only: an Expo config plugin at plugins/with-canonical-prefixes.js injects a CMake flag to work around an NDK short-path bug. It matches a Gradle anchor string, so it can break silently on an Expo upgrade. If a native build starts failing on Windows after a dependency bump, look there.

How to work

One thing per PR. The build is organised in phases in TODO.md; they're a good unit of work. Small and single-purpose beats comprehensive.

Before you open a PR:

npm run typecheck && npm run lint     # app
cd server && npm run typecheck && npm test

Both must pass. There's no CI yet — adding one is itself a welcome PR.

Testing. There's no test runner in the app. Instead there are dev screens that double as the suite, and if you touch the area they cover, run them on a device and say so in the PR:

Screen Covers
/db-test SQLite schema, migrations, query helpers — 11 self-checks
/briefing-test Briefing selection and formatting logic
/llm-test Relay reachability, text and JSON completion, the agent planner
/services-test Permission flows, notification listener, calendar, voice

The server does have a real test file (server/test.mjs, node --test, no network). Logic with a branch, a loop, or a parser in it should leave one runnable check behind. Trivial code doesn't need a test.

The regression that matters most: put the phone in airplane mode and open the home screen. The briefing must still render completely. If a change breaks that, it's broken the anchor habit, and that outranks whatever the change was for.

Code conventions

  • TypeScript strict. No any without a comment justifying it.
  • Comments explain why, not what. The existing code is written this way — a comment that restates the line below it is noise; one that records the constraint that forced an odd shape is worth a lot. Look at src/features/briefing.ts or server/lib/guard.ts for the register.
  • No new dependencies outside the list in AGENT.md §3 without saying why in the PR description. server/ in particular is meant to stay at zero runtime dependencies, and no framework — no Express, no Hono, no Next. Plain handlers are what keep it portable off Vercel.
  • Egress minimisation. Send the LLM the smallest slice that answers the request. Never the whole database, never full notification history.
  • Never log user content — not on the phone, not in the server's runtime logs. Latency and token counts are fine.
  • Glanceable output. Anything user-facing should be readable in about two seconds. This is a wrist-first design discipline even though the MVP is a phone app.

Changing the wire contract

server/contract.ts and src/llm/contract.ts are deliberate duplicates, guarded by CONTRACT_VERSION. If you change one, change both and bump the version — otherwise an older APK gets silent nonsense instead of a clean 400.

They're duplicated rather than shared through a workspace because the Android build is path-sensitive and moving files around it is not worth the risk. If a third consumer ever appears, that's the moment to reconsider.

Pull requests

Include:

  • What changed and why. A sentence each is fine.
  • Which golden rule(s) it touches, if any (AGENT.md §2).
  • How you tested it — which dev screen, which device, or which test.
  • Any new dependency, with a justification.

Commits: imperative subject line, and a body explaining the reasoning rather than restating the diff. git log in this repo shows the house style.

By contributing you agree your contributions are licensed under Apache-2.0, matching the project.

Security

Don't open a public issue for a vulnerability. See SECURITY.md.

Questions

Open an issue. A question that turns out to be a documentation gap is a useful contribution in itself.