docs: add a contributor architecture overview (optional, take what's useful)#540
docs: add a contributor architecture overview (optional, take what's useful)#540horner wants to merge 1 commit into
Conversation
- Add docs/ARCHITECTURE.md mapping the full technical stack (Go+cgo app, Goja config VM, config-api, Svelte UI, build pipeline, CI) for contributors - Link it from the README's 'Building Finicky from source' section - Tailor .github/copilot-instructions.md to Finicky, pointing at the new architecture doc as the single source of truth (DRY)
📝 WalkthroughWalkthroughAdds ChangesContributor Documentation and Copilot Guidance
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/copilot-instructions.md (2)
57-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffRemove or adapt the MCP Browser testing section.
This section describes testing with "MCP Browser in Playwright," which is inappropriate for Finicky — a native macOS application with a Svelte UI embedded via WebKit, not a web app browsable by Playwright. This appears copied from the template reference on line 22 without adaptation. It could mislead contributors into thinking Finicky supports browser-based testing.
- ### 🌐 Testing with MCP Browser - - Use MCP browser in Playwright if available to test functionality - - **Never close the browser** after running MCP browser commands unless explicitly asked - - Let the user interact with the browser after navigation or testing - - Only use `browser_close` when the user specifically requests it🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/copilot-instructions.md around lines 57 - 62, Remove or rewrite the MCP Browser testing guidance in the instructions file so it matches Finicky’s native macOS/WebKit architecture instead of a Playwright web-app workflow. Update the section near the existing testing notes to refer to Finicky-appropriate validation methods and keep any browser-specific wording out of the document, since the current content is copied from a generic template and is misleading.
80-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify that I18N requirements are aspirational, not current practice.
The extensive I18N requirements (RTL support, language selector, localized formats) imply Finicky already has this infrastructure. If the app is English-only currently, these instructions set unrealistic expectations for AI-generated contributions. Consider adding a note that this is a target state or removing until i18n infrastructure exists.
## Internationalization (I18N) + > **Note:** Finicky is currently English-only. The following are target practices for when i18n infrastructure is added. + ### 🌍 Text and Language Support🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/copilot-instructions.md around lines 80 - 91, Clarify the Internationalization section in the copilot instructions so it does not imply Finicky already supports full i18n features. Update the guidance near the I18N heading to state that RTL support, language selection, and localized date/number/currency formats are aspirational or future requirements unless the relevant infrastructure exists. Keep the focus on existing symbols in the doc section itself, such as the “Internationalization (I18N)” and “Localization” headings, and make the language explicitly conditional on current app support.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/copilot-instructions.md:
- Around line 63-66: The HTML & CSS guidelines currently say all styles should
be written in SASS (SCSS), but the finicky-ui app is using plain CSS files, so
update the guidance to match the existing setup. In the
.github/copilot-instructions.md section for CSS guidance, adjust the SASS-First
Approach text to either recommend plain CSS for now or explicitly state Sass
only if support is added, and keep the rest of the styling guidance consistent
with the src/app.css and src/reset.css structure.
---
Nitpick comments:
In @.github/copilot-instructions.md:
- Around line 57-62: Remove or rewrite the MCP Browser testing guidance in the
instructions file so it matches Finicky’s native macOS/WebKit architecture
instead of a Playwright web-app workflow. Update the section near the existing
testing notes to refer to Finicky-appropriate validation methods and keep any
browser-specific wording out of the document, since the current content is
copied from a generic template and is misleading.
- Around line 80-91: Clarify the Internationalization section in the copilot
instructions so it does not imply Finicky already supports full i18n features.
Update the guidance near the I18N heading to state that RTL support, language
selection, and localized date/number/currency formats are aspirational or future
requirements unless the relevant infrastructure exists. Keep the focus on
existing symbols in the doc section itself, such as the “Internationalization
(I18N)” and “Localization” headings, and make the language explicitly
conditional on current app support.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a566255f-6f84-4fa7-bb0a-47c37267b7b1
📒 Files selected for processing (3)
.github/copilot-instructions.mdREADME.mddocs/ARCHITECTURE.md
| ## HTML & CSS Guidelines | ||
| - **Semantic Naming**: Every `<div>` and other structural element must use a meaningful, semantic class name that clearly indicates its purpose or role within the layout. | ||
| - **CSS Simplicity**: Styles should avoid global resets or overrides that affect unrelated components or default browser behavior. Keep changes scoped and minimal. | ||
| - **SASS-First Approach**: All styles should be written in SASS (SCSS) whenever possible. Each component should have its own dedicated SASS file to promote modularity and maintainability. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify styling approach in finicky-ui
fd -i '.*\.s[ac]ss$' packages/finicky-ui/
fd -i '.*\.css$' packages/finicky-ui/ | head -20
cat packages/finicky-ui/package.json | grep -i sass
cat packages/finicky-ui/vite.config.* | grep -i sassRepository: johnste/finicky
Length of output: 219
Align the styling guidance with the current CSS setup. packages/finicky-ui uses plain CSS (src/app.css, src/reset.css), so “All styles should be written in SASS (SCSS)” is misleading unless Sass support is added first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/copilot-instructions.md around lines 63 - 66, The HTML & CSS
guidelines currently say all styles should be written in SASS (SCSS), but the
finicky-ui app is using plain CSS files, so update the guidance to match the
existing setup. In the .github/copilot-instructions.md section for CSS guidance,
adjust the SASS-First Approach text to either recommend plain CSS for now or
explicitly state Sass only if support is added, and keep the rest of the styling
guidance consistent with the src/app.css and src/reset.css structure.
1 participant
First off — thank you for Finicky. 🙏 It totally scratched an itch for me: routing every link to the right browser without thinking about it is exactly the kind of quietly-brilliant utility I wish existed years ago. Really cool project.
While getting my bearings in the codebase I wrote up a contributor-facing overview of the stack, and figured I'd offer it back in case it's useful to others landing here for the first time. Completely optional — cherry-pick, rewrite, or close, no hard feelings at all.
What's here
docs/ARCHITECTURE.md— a one-page map of the technical stack for new contributors: the Go + cgo/Objective-C app, the Goja config VM (config runs without Node),config-api, the Svelte UI, how web assets get embedded, thescripts/build pipeline, and CI. Includes a quick end-to-end flow diagram and a 'where to start' section..github/copilot-instructions.md— opinionated AI-assistant guidance tailored to Finicky. This one is the most 'take it or leave it' of the bunch; it points atARCHITECTURE.mdas the single source of truth (kept DRY) and captures conventions like build-through-scripts/and npm-not-pnpm.Notes
packages/finicky-uideclarespackageManager: pnpmbut the scripts/CI use npm +package-lock.json. I only flagged it in the doc rather than changing anything — happy to follow up separately if you'd like it reconciled.Thanks again for building something genuinely delightful. Feel free to push back on any of this. 🙂
Summary by CodeRabbit