Add Freighter-mobile best practices LLM reference docs#810
Add Freighter-mobile best practices LLM reference docs#810leofelix077 merged 26 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “freighter mobile best practices” documentation skill and supporting context files to help AI agents and contributors navigate Freighter Mobile’s architecture, tooling, and development conventions.
Changes:
- Introduces
llms.txtandCLAUDE.mdas entry-point context/reference docs for the repo. - Adds
docs/skills/freighter-mobile-best-practices/with a skill definition and focused reference guides (architecture, code style, security, WalletConnect, testing, etc.).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| llms.txt | High-level repo index for docs, dev, testing, and key concepts |
| CLAUDE.md | Consolidated AI agent / contributor context (tooling, structure, conventions) |
| docs/skills/freighter-mobile-best-practices/SKILL.md | Skill definition + reference index for best-practices topics |
| docs/skills/freighter-mobile-best-practices/references/architecture.md | Architecture + layering + duck/store patterns reference |
| docs/skills/freighter-mobile-best-practices/references/anti-patterns.md | Common mistakes/anti-pattern guidance |
| docs/skills/freighter-mobile-best-practices/references/code-style.md | Formatting, ESLint/Prettier rules, naming conventions |
| docs/skills/freighter-mobile-best-practices/references/dependencies.md | Dependency management + native dependency workflow |
| docs/skills/freighter-mobile-best-practices/references/error-handling.md | Error normalization + store async patterns + WC error responses |
| docs/skills/freighter-mobile-best-practices/references/git-workflow.md | Branching/commit/PR/release process guidance |
| docs/skills/freighter-mobile-best-practices/references/i18n.md | i18n framework usage + key structure + lint enforcement notes |
| docs/skills/freighter-mobile-best-practices/references/navigation.md | Navigator hierarchy + typing + deep links conventions |
| docs/skills/freighter-mobile-best-practices/references/performance.md | Performance rules/checklist and optimization guidance |
| docs/skills/freighter-mobile-best-practices/references/security.md | Storage tiers + auth/security-sensitive areas overview |
| docs/skills/freighter-mobile-best-practices/references/styling.md | NativeWind/SDS/bottom-sheet/modal styling guidance |
| docs/skills/freighter-mobile-best-practices/references/testing.md | Jest + Maestro structure, commands, and e2e guidance |
| docs/skills/freighter-mobile-best-practices/references/walletconnect.md | WalletConnect architecture, request handling, and validations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rewrite AGENTS.md as the single AI agent entry point: - Add glossary section with domain-specific terminology - Add documentation link index (replaces llms.txt) - Remove sections that duplicate best-practices reference files (code style details, branch conventions, PR instructions) - Keep unique context: repo map, architecture orientation (ducks/nav/WC), security alert list, known complexity/gotchas, pre-submission checklist - Delete llms.txt (content absorbed into AGENTS.md) - Delete CLAUDE.md (content absorbed into AGENTS.md)
Code reviewFound 1 issue:
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Another finding — the Reanimated rule is too absolute given existing legacy Animated usage.
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Another finding — the Hook Return Memoization example has multiple TypeScript errors.
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Another finding — the getItemLayout claim doesn't match codebase reality.
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Another finding — the Provider Layer list mentions a non-existent ThemeProvider.
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Another finding — enableFreeze is presented as if used but isn't.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
….com:stellar/freighter-mobile into lf-add-freighter-mobile-best-practices-skill Co-authored-by: Copilot <copilot@github.com>
|
|
||
| `normalizeError()` feeds directly into Sentry for crash reporting. Always | ||
| normalize errors before sending to Sentry to ensure consistent, actionable | ||
| reports. |
There was a problem hiding this comment.
[Comment 22 — Suggestion] Sentry Integration section contradicts the earlier guidance
This says:
"
normalizeError()feeds directly into Sentry for crash reporting. Always normalize errors before sending to Sentry to ensure consistent, actionable reports."
But the earlier "Error Normalization" section (lines 34-35) says:
"Use
logger.error()to report errors — it normalizes and forwards to Sentry internally. Do not callSentry.captureException()directly."
And the Rules section at line 159:
"Never call
Sentry.captureException()directly — go throughlogger.error()..."
The phrasing in the Sentry Integration section implies the agent should "send to Sentry" themselves after normalizing — which contradicts the rule against calling Sentry directly. An agent reading the bottom section in isolation might still try to call Sentry.captureException(normalizedError).
Suggest collapsing the redundancy:
-## Sentry Integration
-
-`normalizeError()` feeds directly into Sentry for crash reporting. Always
-normalize errors before sending to Sentry to ensure consistent, actionable
-reports.
+## Sentry Integration
+
+Sentry receives normalized errors automatically when you call `logger.error()` —
+the logger normalizes via `normalizeError()` and forwards internally. You don't
+need to call Sentry yourself.Or delete the section entirely — the same info is already covered in "Error Normalization" and the Rules section.
* fix(troubleshooting): add emulator recreation tip for persistent memory issues Android Studio sometimes doesn't apply RAM changes to existing AVDs reliably; recreating the emulator is the definitive fix. * cleanup runtime information and clipboard not pasting * Update code review troubleshooting comments * cleanup references naming * clean up troubleshooting guide for stable xcode and IDE config * breakdown commands on troubleshooting guide for better readability * remove xcode 26 regression issue
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
Just one tiny reminder before merging.
| @@ -0,0 +1,442 @@ | |||
| # Troubleshooting Guide: Freighter Mobile | |||
|
|
|||
| _Last updated: 2026-04-08_ | |||
There was a problem hiding this comment.
[Comment 23 — Nit] Update the "Last updated" date before merging
This says 2026-04-08 but the guide has been iterated through 2026-04-29. Worth bumping to today's date so the staleness signal is accurate.
Benchmark Report — 4-Config Comparison
Per-Eval BreakdownEval 1 — code-style-naming
Eval 2 — architecture-zustand
Eval 3 — performance-flatlist
Eval 4 — security-walletconnect
Post-benchmark doc updates (commit 0d9541b)Three changes were committed after the Refs-only/With-skill run, addressing Change Addresses Outstanding from proposed path forward:
|
|
@CassioMG unlike the extension, mobile doesnt seem to have much difference between skill and bare refs, so I moved them out of the skills folder and left only the agents.md file |
CassioMG
left a comment
CassioMG
left a comment
There was a problem hiding this comment.
One small consistency finding from the final pass.
| ```tsx | ||
| if (hasRespondedRef.current) return; | ||
| hasRespondedRef.current = true; | ||
| await walletKit.respondSessionRequest({ ... }); |
There was a problem hiding this comment.
[Comment 24 — Suggestion] Use the rejectSessionRequest helper here for consistency with error-handling.md
The hasRespondedRef anti-replay example here still uses the raw walletKit.respondSessionRequest({ ... }), but error-handling.md:165-168 (updated in commit 06ffd5cb) uses the rejectSessionRequest helper for the exact same pattern:
// error-handling.md
if (hasRespondedRef.current) return;
hasRespondedRef.current = true;
await rejectSessionRequest({ sessionRequest, message });The helper was added in Comment 15 to keep callers from re-implementing the JSON-RPC error structure. Worth aligning walletconnect.md to the same pattern so both files show consistent guidance.
Suggested:
+import { rejectSessionRequest } from "helpers/walletKitUtil";
+
if (hasRespondedRef.current) return;
hasRespondedRef.current = true;
-await walletKit.respondSessionRequest({ ... });
+await rejectSessionRequest({ sessionRequest, message });
@leofelix077 removing the SKILL sounds good to me, thanks for running the benchmark. I think the PR should be good to merge as soon as the 3 open comments are resolved (here, here and here). Could you please update the PR title and description to reflect the current state of the PR now that it's not adding a SKILL? Thanks |
leofelix077
changed the title
|
@CassioMG made the adjustments. will mege it then |
Skill Eval:
freighter-mobile-best-practices— Benchmark ReportDate: 2026-04-09
Iterations: 1
Repo: stellar/freighter-mobile
Model: Claude Opus 4.6
Summary
on further passes both get close to 100%, as Claude picks up automatically on the rules and the entry points
Assertion Results (aggregated across 3 iterations)
architecture-screen
architecture-zustand
code-style-hook
code-style-naming
err-handling-retry
err-handling-zustand
i18n-settings
nav-typed-routes
performance-flatlist
performance-selectors
security-storage
security-walletconnect
styling-card
testing-zustand