ci: add weeder check for unused code detection

[tonyalaribe]

Closes #

How to test

Checklist

• Make sure you have described your changes and added all relevant screenshots or data.
• Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests).
• Make sure to add/update documentation regarding your changes (or request one from the team).
• You are NOT deprecating/removing a feature.

[claude]

Code Review - PR #243: CI Weeder Check

Summary

This PR adds weeder (unused code detection) to the CI pipeline. The implementation is solid overall, with good caching strategy and proper artifact management.

✅ Strengths

• Good separation of concerns: Weeder job properly depends on build job
• Efficient caching: Weeder binary cached with appropriate key (weeder-2.9.0-ghc-9.12.2)
• HIE files generated: Correct use of -fwrite-ide-info -hiedir=.hiefiles
• Artifact retention: 1-day retention is appropriate for PR artifacts
• Config exists: config/weeder.toml already configured

🔧 Suggested Improvements

1. Reduce artifact size (Performance)

Uploading entire dist-newstyle (~hundreds of MB) is excessive. Weeder only needs specific files:

- name: Upload build artifacts for weeder
  uses: actions/upload-artifact@v4
  with:
    name: weeder-artifacts
    path: |
      .hiefiles
      dist-newstyle/**/*.conf
    retention-days: 1

Then combine downloads:

- name: Download build artifacts
  uses: actions/download-artifact@v4
  with:
    name: weeder-artifacts

Impact: Faster upload/download, less storage, reduced CI time.

2. Consolidate GHC options (Code Quality)

Line 72 duplicates GHC options from line 66. Use single build command - the -O0 -j8 from configure already applies globally.

3. Add validation (Reliability)

Add check before running weeder:

- name: Run weeder
  run: |
    if [ ! -d ".hiefiles" ]; then
      echo "Error: HIE files not found"
      exit 1
    fi
    ~/.local/bin/weeder --config config/weeder.toml

4. Minor: PATH vs absolute path (Code Style)

Instead of ~/.local/bin/weeder, add to PATH for cleaner code:

- name: Run weeder
  run: |
    export PATH="$HOME/.local/bin:$PATH"
    weeder --config config/weeder.toml

📊 Test Coverage

• ✅ Weeder config exists and properly configured
• ⚠️ Consider adding a test case with known dead code to verify weeder actually runs

🔒 Security

• ✅ No security concerns
• ✅ Artifacts have appropriate short retention
• ✅ No secrets exposed

📝 Other Notes

• PR description is empty - please add context about why weeder is being added
• Checklist items not completed

Overall: Good implementation. Main suggestion is reducing artifact size for better performance. Ready to merge with or without suggested improvements.

[claude]

Code Review: Weeder CI Integration

Summary

This PR adds weeder integration to detect unused code in CI. Good initiative for code quality.

Issues & Recommendations

Critical: Artifact Upload Size 🔴

Lines 74-81: Uploading entire dist-newstyle/**/*.conf will create massive artifacts (potentially GBs).

• Weeder only needs package configuration, not all build artifacts
• Fix: Use specific path: dist-newstyle/packagedb/ghc-9.12.2/*.conf or similar

Redundant Validation ⚠️

Lines 235-238: The HIE files check is unnecessary:

• download-artifact already fails if artifact missing
• Extra validation adds no value
• Remove: Delete the conditional check, let the action fail naturally

Missing Error Handling 🔴

Line 239: Weeder execution has no output capture or interpretation:

• Weeder failures should be clear and actionable
• Add: || { echo "::error::Weeder found unused code"; exit 1; }

GHC Options Duplication ⚠️

Lines 66 + 72: GHC options specified in both configure and build:

• --ghc-options already set in configure (line 66)
• Simplify: Remove from line 72, use: cabal build all -j4
• Then add HIE-specific options only: --ghc-options="-fwrite-ide-info -hiedir=.hiefiles"

Optimization Opportunity 💡

Lines 214-226: Weeder installation could be faster:

• Consider using pre-built binary from GitHub releases
• Or add to project's tool dependencies in cabal.project
• Current approach rebuilds weeder on cache miss

Code Succinctness

The yaml could be more concise:

# Lines 233-239 current:
- name: Run weeder
  run: |
    if [ ! -d ".hiefiles" ]; then
      echo "Error: HIE files not found"
      exit 1
    fi
    ~/.local/bin/weeder --config config/weeder.toml

# Suggested:
- name: Run weeder
  run: ~/.local/bin/weeder --config config/weeder.toml

Testing Recommendations

• Verify artifact size after fixing path (should be <10MB)
• Test weeder with intentionally unused code to ensure it fails CI
• Document in weeder.toml what code patterns are considered roots

Security

✅ No security concerns. Using official GitHub actions and specific weeder version.

Performance

• HIE file generation adds ~5-10% to build time (acceptable)
• Artifact upload/download could be slow if not fixed (see above)
• Weeder execution itself is fast (<30s typically)

---

Priority: Fix artifact upload path and remove redundant validation before merging.