From 6c830c8eb1e903e45e31fe39203ed60258109fa4 Mon Sep 17 00:00:00 2001 From: JeffG <202880+erskingardner@users.noreply.github.com> Date: Wed, 18 Feb 2026 10:04:20 +0100 Subject: [PATCH] chore(ci): add PR labeler and improve coverage reporting (#304) * chore(ci): add PR labeler and improve coverage reporting * chore(ci): split lint test and coverage jobs * fix(ci): dedupe coverage history entries by sha * fix(ci): use github.workspace in cache paths * fix(ci): run flutter tests once for coverage * fix(ci): consume coverage from shared test artifact * fix(ci): align coverage guidance and add checkout * fix(ci): checkout before downloading coverage artifact * chore(coderabbit): dedupe title ignore keywords * docs: refine widget naming conventions into three categories Encode reviewer feedback distinguishing design system widgets (Wn prefix), complex reusable widgets (no prefix), and screen-scoped widgets (screen name prefix) in both .coderabbit.yaml and AGENTS.md. * fix(ci): cache lcov apt package to avoid repeated installs --- .coderabbit.yaml | 198 +++++++++++++++++++++++- .github/labeler.yml | 42 +++++ .github/workflows/ci.yml | 255 ++++++++++++++++++++++++++++--- .github/workflows/pr-labeler.yml | 20 +++ AGENTS.md | 15 +- 5 files changed, 501 insertions(+), 29 deletions(-) create mode 100644 .github/labeler.yml create mode 100644 .github/workflows/pr-labeler.yml diff --git a/.coderabbit.yaml b/.coderabbit.yaml index edd2d67..c89d5e7 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,15 +1,211 @@ # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json language: "en-US" early_access: false + reviews: profile: "assertive" request_changes_workflow: false high_level_summary: true + high_level_summary_placeholder: "@coderabbitai summary" poem: false review_status: true - collapse_walkthrough: false + review_details: true + collapse_walkthrough: true + sequence_diagrams: true + changed_files_summary: true + estimate_code_review_effort: true + assess_linked_issues: true + related_prs: true + + # --- Reviewer suggestions ---------------------------------------------- + suggested_reviewers: true + + # --- Path filters (skip generated & non-reviewable files) --------------- + path_filters: + # Generated Flutter-Rust bridge code (auto-generated, DO NOT EDIT) + - "!lib/src/rust/frb_generated.dart" + - "!lib/src/rust/frb_generated.io.dart" + - "!rust/src/frb_generated.rs" + # Generated localizations + - "!lib/l10n/generated/**" + # Freezed generated code + - "!**/*.freezed.dart" + # Widgetbook generated directories file + - "!widgetbook/lib/main.directories.g.dart" + # Platform-generated plugin registrants + - "!**/generated_plugin_registrant.*" + - "!**/generated_plugins.cmake" + - "!**/GeneratedPluginRegistrant.*" + # Lock files + - "!pubspec.lock" + - "!rust/Cargo.lock" + # Assets (images, fonts) + - "!assets/images/**" + - "!assets/fonts/**" + + # --- Path-specific review instructions ---------------------------------- + path_instructions: + - path: "lib/screens/**" + instructions: | + This is a Flutter screen (full-page widget). + Architecture rules: + - Screens should WATCH Riverpod providers for shared state + - Use flutter_hooks for ephemeral/local state (NOT StatefulWidget) + - Pass data to hooks, not refs + - Use flutter_screenutil (.w, .h, .sp, .r) for all size values + - Widgets should use const constructors where possible + - No comments except for truly complex logic + - When a widget is extracted from a screen and only used in that one + screen, it should be prefixed with the screen name (e.g. + ChatListTile for a widget only used in the chat list screen). + These are screen-scoped widgets and do NOT use the Wn prefix. + + - path: "lib/widgets/**" + instructions: | + This is a reusable widget. + There are two kinds of reusable widgets: + + 1. Design system widgets — simple, presentational widgets that match + the Figma design system in name and structure. They have Widgetbook + stories, contain only presentational logic, and do NOT have + translations or Rust API calls. + - File MUST be prefixed with wn_ (e.g. wn_filled_button.dart) + - Class MUST be prefixed with Wn (e.g. WnFilledButton) + + 2. Complex reusable widgets — used across multiple screens but contain + translations, hooks with Rust API calls, or other complex logic + that makes them harder to display in Widgetbook. + - These do NOT use the wn_/Wn prefix + - Example: OnboardingCarousel (used in multiple screens, has + translations and a page controller inside) + + General rules for all widgets in this directory: + - Use const constructors where possible + - Use flutter_screenutil (.w, .h, .sp, .r) for all dimensions + - Avoid StatefulWidget — prefer hooks for local state + - No comments except for truly complex logic + + - path: "lib/providers/**" + instructions: | + This is a Riverpod provider (shared app-wide state). + Rules: + - Files must end with _provider.dart + - Provider variables must end with Provider (e.g. authProvider) + - Don't duplicate logic from the Rust crate — whitenoise is source of truth + - Don't cache data that whitenoise already persists in its local DB + + - path: "lib/hooks/**" + instructions: | + This is a flutter_hooks hook (ephemeral widget-local state). + Rules: + - Files must be prefixed with use_ (e.g. use_chat_list.dart) + - Hook functions must start with use (e.g. useChatList()) + - Hooks receive data as parameters, not widget refs + - Ensure proper cleanup/dispose of subscriptions and resources + + - path: "lib/services/**" + instructions: | + Services are stateless operations (API calls, etc.). + They should not hold state — that belongs in providers. + Check that they don't duplicate logic from whitenoise-rs. + + - path: "rust/src/api/**" + instructions: | + This is the Rust API layer exposed to Flutter via flutter_rust_bridge. + Rules: + - Functions use #[frb] attribute for bridge generation + - Structs use #[frb(non_opaque)] for Flutter compatibility + - Errors must be wrapped in the ApiError enum using thiserror + - This is a thin wrapper around the whitenoise crate — keep it thin + - No unwrap() in non-test code; use proper error handling + - Check for correct async patterns + + - path: "test/**" + instructions: | + IMPORTANT: CI enforces coverage regression (coverage must never decrease). It does not enforce a fixed 95% minimum threshold. + Rules: + - Test files mirror source structure with _test.dart suffix + - Use helpers from test/test_helpers.dart (setUpTestView, mountTestApp, etc.) + - Mock Rust API using RustLib.initMock(api: mockApi) + - Always extend MockWnApi from test/mocks/mock_wn_api.dart + - Prefer find.byKey() over find.byIcon() for widget testing + - Use valid 64-char hex strings for pubkeys, not dummy values like 'abc' + - Tests must be deterministic — no external service dependencies + + - path: "**/*.arb" + instructions: | + These are localization files. Check for: + - Consistent key naming across all locale files + - Proper ICU message format for plurals/gender + - No hardcoded strings that should be localized + + - path: "scripts/**" + instructions: "Build and CI scripts. Check for portability and proper error handling." + + # --- Auto review settings ----------------------------------------------- auto_review: enabled: true drafts: true + auto_incremental_review: true + ignore_title_keywords: + - "WIP" + - "DO NOT MERGE" + base_branches: [] + + # --- Pre-merge checks --------------------------------------------------- + pre_merge_checks: + title: + mode: "warning" + requirements: > + Use a descriptive title. Preferred format: type(scope): description + where type is feat/fix/chore/docs/refactor/test and scope is optional. + Examples: "feat: add group creation flow", "fix(auth): handle relay timeout" + description: + mode: "warning" + issue_assessment: + mode: "warning" + + # --- Finishing touches --------------------------------------------------- + finishing_touches: + docstrings: + enabled: false # Project prefers self-explanatory code over docstrings + unit_tests: + enabled: true + + # --- Tools -------------------------------------------------------------- + tools: + # Secret scanning (important for crypto/key-handling project) + gitleaks: + enabled: true + trufflehog: + enabled: true + # Dart/Flutter analysis handled by analysis_options.yaml in-repo + # YAML linting for config files + yamllint: + enabled: true + # Markdown linting for docs + markdownlint: + enabled: true + # Shell script checking + shellcheck: + enabled: true + # Not relevant for this project + biome: + enabled: false + ruff: + enabled: false + phpstan: + enabled: false + phpmd: + enabled: false + phpcs: + enabled: false + golangci-lint: + enabled: false + hadolint: + enabled: false + checkov: + enabled: false + chat: auto_reply: true diff --git a/.github/labeler.yml b/.github/labeler.yml new file mode 100644 index 0000000..d65ea34 --- /dev/null +++ b/.github/labeler.yml @@ -0,0 +1,42 @@ +flutter-ui: + - changed-files: + - any-glob-to-any-file: + - 'lib/screens/**' + - 'lib/widgets/**' + - 'lib/theme.dart' + - 'assets/**' + +flutter: + - changed-files: + - any-glob-to-any-file: + - 'lib/**' + - 'pubspec.yaml' + - 'analysis_options.yaml' + - 'l10n.yaml' + +rust: + - changed-files: + - any-glob-to-any-file: + - 'rust/**' + - 'flutter_rust_bridge.yaml' + - 'lib/src/rust/**' + +tests: + - changed-files: + - any-glob-to-any-file: + - 'test/**' + - 'rust/**/tests/**' + +ci: + - changed-files: + - any-glob-to-any-file: + - '.github/workflows/**' + - 'scripts/**' + - 'justfile' + +docs: + - changed-files: + - any-glob-to-any-file: + - 'README.md' + - 'CHANGELOG.md' + - '**/*.md' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 054f8c1..adb5705 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,11 +7,9 @@ on: branches: [master] jobs: - lint-and-test: - name: Lint and test + lint: + name: Lint runs-on: ubuntu-latest - outputs: - coverage: ${{ steps.coverage.outputs.percentage }} steps: - name: Checkout code @@ -36,7 +34,7 @@ jobs: with: path: | ~/.pub-cache - ${{ runner.workspace }}/.pub-cache + ${{ github.workspace }}/.pub-cache key: ${{ runner.os }}-pub-cache-${{ hashFiles('**/pubspec.lock') }} restore-keys: | ${{ runner.os }}-pub-cache- @@ -75,7 +73,41 @@ jobs: - name: Flutter analyze (widgetbook) run: cd widgetbook && flutter analyze --fatal-infos - - name: Run Flutter tests + test: + name: Test + runs-on: ubuntu-latest + outputs: + coverage: ${{ steps.coverage.outputs.percentage }} + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Flutter + uses: subosito/flutter-action@v2 + with: + flutter-version: '3.38.4' + channel: 'stable' + cache: true + + - name: Cache Flutter dependencies + uses: actions/cache@v4 + with: + path: | + ~/.pub-cache + ${{ github.workspace }}/.pub-cache + key: ${{ runner.os }}-pub-cache-${{ hashFiles('**/pubspec.lock') }} + restore-keys: | + ${{ runner.os }}-pub-cache- + + - name: Install Flutter dependencies + run: | + flutter pub get + cd widgetbook && flutter pub get + + - name: Run Flutter tests with coverage run: flutter test --coverage - name: Extract coverage percentage @@ -86,34 +118,103 @@ jobs: echo "percentage=$COVERAGE" >> "$GITHUB_OUTPUT" echo "Current coverage: $COVERAGE%" + - name: Install lcov (cached) + uses: awalsh128/cache-apt-pkgs-action@v1 + with: + packages: lcov + version: 1.0 + + - name: Generate coverage summary and HTML report + run: | + mkdir -p coverage + + LINES_HIT=$(grep -E "^LH:" coverage/lcov.info | awk -F: '{sum+=$2} END {print sum+0}') + TOTAL_LINES=$(grep -E "^LF:" coverage/lcov.info | awk -F: '{sum+=$2} END {print sum+0}') + + { + echo "Coverage summary" + echo "================" + echo "Coverage: ${{ steps.coverage.outputs.percentage }}%" + echo "Lines hit: $LINES_HIT" + echo "Lines found: $TOTAL_LINES" + } > coverage/summary.txt + + genhtml coverage/lcov.info --output-directory coverage/html + + - name: Upload HTML coverage report + uses: actions/upload-artifact@v4 + with: + name: coverage-report-html + path: coverage/html/ + retention-days: 90 + + - name: Upload lcov coverage report + uses: actions/upload-artifact@v4 + with: + name: coverage-report-lcov + path: coverage/lcov.info + retention-days: 90 + + - name: Upload coverage summary + uses: actions/upload-artifact@v4 + with: + name: coverage-summary + path: coverage/summary.txt + retention-days: 90 + coverage: name: Coverage if: github.event_name == 'pull_request' - needs: lint-and-test + needs: [lint, test] runs-on: ubuntu-latest permissions: + actions: read contents: read pull-requests: write steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Download coverage artifact from test job + uses: actions/download-artifact@v4 + with: + name: coverage-report-lcov + path: coverage-input + + - name: Read coverage percentage from shared artifact + id: current_coverage + run: | + chmod +x scripts/check-coverage.sh + LCOV_FILE=$(find coverage-input -type f -name 'lcov.info' | head -n 1) + + if [ -z "$LCOV_FILE" ]; then + echo "::error::Could not find lcov.info in downloaded coverage artifact." + exit 1 + fi + + CURRENT=$(./scripts/check-coverage.sh "$LCOV_FILE") + echo "percentage=$CURRENT" >> "$GITHUB_OUTPUT" + echo "Current coverage from shared artifact: $CURRENT%" + - name: Get master branch coverage id: master_coverage run: | echo "🔍 Searching for baseline from master branch..." - # Get the latest completed CI workflow run ID from master + # Get the latest successful CI workflow run ID from master RUN_ID=$(gh run list \ --repo ${{ github.repository }} \ --workflow=ci.yml \ --branch master \ - --status completed \ + --status success \ --limit 1 \ --json databaseId \ --jq '.[0].databaseId') if [ -z "$RUN_ID" ] || [ "$RUN_ID" = "null" ]; then echo "baseline=0" >> "$GITHUB_OUTPUT" - echo "⚠️ No completed CI run found on master" + echo "⚠️ No successful CI run found on master" exit 0 fi @@ -140,10 +241,11 @@ jobs: env: GH_TOKEN: ${{ github.token }} - - name: Compare coverage + - name: Compute coverage delta if: steps.master_coverage.outputs.baseline != '0' + id: coverage_delta run: | - CURRENT=${{ needs.lint-and-test.outputs.coverage }} + CURRENT=${{ steps.current_coverage.outputs.percentage }} BASELINE=${{ steps.master_coverage.outputs.baseline }} echo "=== Coverage Comparison ===" @@ -151,49 +253,152 @@ jobs: echo "This PR: $CURRENT%" DIFF=$(awk "BEGIN {printf \"%.2f\", $CURRENT - $BASELINE}") - if (( $(awk "BEGIN {print ($CURRENT < $BASELINE)}") )); then + STATUS='decreased' echo "❌ Coverage decreased by ${DIFF#-}%" - echo "::error::Coverage regression detected. Coverage decreased from $BASELINE% to $CURRENT% ($DIFF%)" - exit 1 elif (( $(awk "BEGIN {print ($CURRENT > $BASELINE)}") )); then + STATUS='increased' echo "✅ Coverage improved by $DIFF%" else + STATUS='same' echo "✅ Coverage maintained at $BASELINE%" fi + echo "current=$CURRENT" >> "$GITHUB_OUTPUT" + echo "baseline=$BASELINE" >> "$GITHUB_OUTPUT" + echo "diff=$DIFF" >> "$GITHUB_OUTPUT" + echo "status=$STATUS" >> "$GITHUB_OUTPUT" + + - name: Write coverage step summary + if: always() + run: | + if [ "${{ steps.master_coverage.outputs.baseline }}" = "0" ]; then + { + echo "## Coverage" + echo "" + echo "Could not find a master baseline artifact, so comparison was skipped." + echo "Current PR coverage: **${{ steps.current_coverage.outputs.percentage }}%**" + } >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + + CURRENT="${{ steps.coverage_delta.outputs.current }}" + BASELINE="${{ steps.coverage_delta.outputs.baseline }}" + DIFF="${{ steps.coverage_delta.outputs.diff }}" + STATUS="${{ steps.coverage_delta.outputs.status }}" + SIGN="" + + if [ "$STATUS" = "increased" ]; then + SIGN="+" + fi + + { + echo "## Coverage" + echo "" + echo "| Baseline (master) | PR | Delta |" + echo "| --- | --- | --- |" + echo "| ${BASELINE}% | ${CURRENT}% | ${SIGN}${DIFF}% |" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Fail on coverage regression + if: steps.master_coverage.outputs.baseline != '0' && steps.coverage_delta.outputs.status == 'decreased' + run: | + echo "::error::Coverage regression detected. Coverage decreased from ${{ steps.coverage_delta.outputs.baseline }}% to ${{ steps.coverage_delta.outputs.current }}% (${{ steps.coverage_delta.outputs.diff }}%)" + exit 1 + - name: Comment PR with coverage change if: always() && steps.master_coverage.outputs.baseline != '0' uses: actions/github-script@v7 with: script: | - const current = parseFloat('${{ needs.lint-and-test.outputs.coverage }}'); + const current = parseFloat('${{ steps.current_coverage.outputs.percentage }}'); const baseline = parseFloat('${{ steps.master_coverage.outputs.baseline }}'); - const diff = (current - baseline).toFixed(2); + const rawDiff = current - baseline; + const displayDiff = rawDiff.toFixed(2); + const sha = context.sha.substring(0, 7); + const now = new Date().toISOString().replace('T', ' ').substring(0, 19) + ' UTC'; + const emoji = rawDiff > 0 ? ':white_check_mark:' : rawDiff < 0 ? ':x:' : ':white_check_mark:'; + const sign = rawDiff > 0 ? '+' : ''; - let emoji = current > baseline ? '✅' : current < baseline ? '❌' : '➡️'; + const marker = ''; + const historyStart = ''; + const historyEnd = ''; - const body = `## ${emoji} Coverage: ${baseline}% → ${current}% (${diff > 0 ? '+' : ''}${diff}%)`; - - github.rest.issues.createComment({ + const { data: comments } = await github.rest.issues.listComments({ issue_number: context.issue.number, owner: context.repo.owner, repo: context.repo.repo, - body: body }); + const existing = comments.find(c => c.body && c.body.includes(marker)); + let historyEntries = []; + + if (existing) { + const hStart = existing.body.indexOf(historyStart); + const hEnd = existing.body.indexOf(historyEnd); + + if (hStart !== -1 && hEnd !== -1) { + const historyBlock = existing.body.substring( + hStart + historyStart.length, hEnd + ); + historyEntries = historyBlock + .split('\n') + .filter(line => line.trim().startsWith('- ')); + } + } + + const shaEntryPrefix = `- \`${sha}\``; + historyEntries = historyEntries.filter( + line => !line.trim().startsWith(shaEntryPrefix) + ); + + historyEntries.push( + `- \`${sha}\` ${now} — **${current}%** (${sign}${displayDiff}% vs base)` + ); + + const body = [ + `## ${emoji} Coverage: ${baseline}% → ${current}% (${sign}${displayDiff}%)`, + '', + '
', + 'History', + '', + historyStart, + ...historyEntries, + historyEnd, + '', + '
', + '', + marker, + ].join('\n'); + + if (existing) { + await github.rest.issues.updateComment({ + comment_id: existing.id, + owner: context.repo.owner, + repo: context.repo.repo, + body: body, + }); + } else { + await github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: body, + }); + } + coverage-baseline: name: Coverage Baseline if: github.event_name == 'push' - needs: lint-and-test + needs: test runs-on: ubuntu-latest steps: - name: Save coverage baseline run: | mkdir -p coverage-baseline - echo "${{ needs.lint-and-test.outputs.coverage }}" > coverage-baseline/coverage-baseline.txt - echo "Saved baseline: ${{ needs.lint-and-test.outputs.coverage }}%" + echo "${{ needs.test.outputs.coverage }}" > coverage-baseline/coverage-baseline.txt + echo "Saved baseline: ${{ needs.test.outputs.coverage }}%" - name: Upload coverage baseline uses: actions/upload-artifact@v4 diff --git a/.github/workflows/pr-labeler.yml b/.github/workflows/pr-labeler.yml new file mode 100644 index 0000000..3c06280 --- /dev/null +++ b/.github/workflows/pr-labeler.yml @@ -0,0 +1,20 @@ +name: PR Labeler + +on: + pull_request_target: + types: [opened, synchronize, reopened, ready_for_review] + +permissions: + contents: read + pull-requests: write + +jobs: + label: + name: Apply PR labels + runs-on: ubuntu-latest + steps: + - name: Apply labels from changed files + uses: actions/labeler@v6 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + sync-labels: true diff --git a/AGENTS.md b/AGENTS.md index 031e2a9..a711f27 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ whitenoise/ │ ├── providers/ # Riverpod providers (shared state) │ ├── hooks/ # Flutter hooks (ephemeral state) │ ├── screens/ # Full-page UI components -│ ├── widgets/ # Reusable components (prefixed wn_) +│ ├── widgets/ # Reusable components (see Widget Naming) │ ├── services/ # Stateless operations (API calls) │ ├── extensions/ # Dart extensions │ ├── utils/ # Utility functions @@ -165,8 +165,17 @@ rust tests... ✓ ### Widget Naming -- Reusable widgets prefixed with `wn_` (e.g., `wn_filled_button.dart`) -- Widget class names use `Wn` prefix (e.g., `WnFilledButton`) +There are three categories of widgets with different naming rules: + +1. **Design system widgets** — Simple, presentational widgets that match the Figma design system. They have Widgetbook stories, no translations, and no Rust API calls. + - File prefixed with `wn_` (e.g., `wn_filled_button.dart`) + - Class prefixed with `Wn` (e.g., `WnFilledButton`) + +2. **Complex reusable widgets** — Used across multiple screens but contain translations, hooks with Rust API calls, or other complex logic. + - No `wn_`/`Wn` prefix (e.g., `onboarding_carousel.dart` / `OnboardingCarousel`) + +3. **Screen-scoped widgets** — Extracted from a single screen for simplicity, only used in that one screen. + - Prefixed with the screen name (e.g., `ChatListTile` for a widget only used in the chat list screen) ### Hook Naming