Phase 07 Context

Goal

Not available

Codebase Map Available

Codebase mapping exists in .vbw-planning/codebase/. Key files:

• ARCHITECTURE.md
• CONCERNS.md
• PATTERNS.md
• DEPENDENCIES.md
• STRUCTURE.md
• CONVENTIONS.md
• TESTING.md
• STACK.md

Read CONVENTIONS.md, PATTERNS.md, STRUCTURE.md, and DEPENDENCIES.md first to bootstrap codebase understanding.

Changed Files (Delta)

• .vbw-planning/config.json
• .vbw-planning/milestones/03-coverage-analysis-quick-wins-critical-path-test-co/STATE.md
• .vbw-planning/phases/02-feed-reliability/01-SUMMARY.md
• .vbw-planning/phases/02-feed-reliability/02-SUMMARY.md
• .vbw-planning/phases/02-feed-reliability/03-SUMMARY.md
• .vbw-planning/phases/02-feed-reliability/04-SUMMARY.md
• .vbw-planning/phases/03-dashboard-pagination/03-04-SUMMARY.md
• .vbw-planning/phases/05-simplify-source-status/01-SUMMARY.md
• .vbw-planning/phases/05-simplify-source-status/03-SUMMARY.md
• .vbw-planning/phases/05-simplify-source-status/05-SUMMARY.md
• .vbw-planning/ROADMAP.md
• app/assets/builds/source_monitor/application.css
• test/dummy/config/puma.rb
• test/dummy/db/schema.rb
• test/dummy/Gemfile.lock

Code Slices

.vbw-planning/config.json (51 lines, first 30 shown)

{
  "effort": "thorough",
  "autonomy": "standard",
  "auto_commit": true,
  "planning_tracking": "manual",
  "auto_push": "never",
  "verification_tier": "standard",
  "skill_suggestions": true,
  "auto_install_skills": false,
  "discovery_questions": true,
  "context_compiler": true,
  "visual_format": "unicode",
  "max_tasks_per_plan": 5,
  "prefer_teams": "auto",
  "branch_per_milestone": false,
  "plain_summary": true,
  "active_profile": "default",
  "custom_profiles": {},
  "model_profile": "quality",
  "model_overrides": {},
  "agent_max_turns": {
    "scout": 15,
    "qa": 25,
    "architect": 30,
    "debugger": 80,
    "lead": 50,
    "dev": 75
  },
  "qa_skip_agents": [
    "docs"

.vbw-planning/milestones/03-coverage-analysis-quick-wins-critical-path-test-co/STATE.md (64 lines, first 30 shown)

<!-- VBW STATE TEMPLATE (ARTF-05) -- Session dashboard, auto-updated -->
<!-- Updated after each plan completion and at checkpoints -->

# Project State

## Project Reference

See: .vbw-planning/PROJECT.md (updated 2026-02-09)

**Core value:** Drop-in Rails engine for feed monitoring, content scraping, and operational dashboards.
**Current focus:** All phases complete

## Current Position

Phase: 4 of 4 (Code Quality & Conventions Cleanup)
Plan: 3 of 3 in current phase
Status: Built
Last activity: 2026-02-10 -- Phase 4 complete (3/3 plans done)

Progress: [##########] 100%

## Codebase Profile

- **Total source files:** 535
- **Primary language:** Ruby (330 files)
- **Templates:** ERB (48 files)
- **Tests:** 137 test files detected
- **Test suite:** 841 runs, 2776 assertions, 0 failures (up from 473 runs in Phase 1)
- **Coverage:** 86.97% line, 58.84% branch (510 uncovered lines, down from 2117)
- **CI/CD:** GitHub Actions (1 workflow)

.vbw-planning/ROADMAP.md (135 lines, first 30 shown)

# Roadmap

**Milestone:** rails-audit-and-modal

## Phases

- [x] Phase 01: Quick Wins & Security
- [x] Phase 02: Model Layer Hardening
- [x] Phase 03: Controller & Route Refactoring
- [x] Phase 04: Job & Pipeline Reliability
- [x] Phase 05: View Layer Extraction
- [x] Phase 06: Test Infrastructure
- [ ] Phase 07: Rails Audit Round 2

## Phase Details

### Phase 01: Quick Wins & Security

**Goal:** Fix high-priority security issues and low-effort quick wins from the Rails audit (findings M1, M3, M5, M12, C6, C9, V2, V8, V10, V11).

**Success Criteria:**
- ImportHistoryDismissalsController scoped to current user
- DashboardController uses explicit parameter allowlist (no .permit!)
- ScrapeLog has by_source, by_status, by_item scopes
- Scrape status badge logic extracted to helper
- MutationObserver disconnect leak fixed

**Plans:**
- [x] Plan 01: Security & Controller Fixes
- [x] Plan 02: Model & Job Fixes

app/assets/builds/source_monitor/application.css (2265 lines, first 30 shown)

*, ::before, ::after {
  --tw-border-spacing-x: 0;
  --tw-border-spacing-y: 0;
  --tw-translate-x: 0;
  --tw-translate-y: 0;
  --tw-rotate: 0;
  --tw-skew-x: 0;
  --tw-skew-y: 0;
  --tw-scale-x: 1;
  --tw-scale-y: 1;
  --tw-pan-x:  ;
  --tw-pan-y:  ;
  --tw-pinch-zoom:  ;
  --tw-scroll-snap-strictness: proximity;
  --tw-gradient-from-position:  ;
  --tw-gradient-via-position:  ;
  --tw-gradient-to-position:  ;
  --tw-ordinal:  ;
  --tw-slashed-zero:  ;
  --tw-numeric-figure:  ;
  --tw-numeric-spacing:  ;
  --tw-numeric-fraction:  ;
  --tw-ring-inset:  ;
  --tw-ring-offset-width: 0px;
  --tw-ring-offset-color: #fff;
  --tw-ring-color: rgb(59 130 246 / 0.5);
  --tw-ring-offset-shadow: 0 0 #0000;
  --tw-ring-shadow: 0 0 #0000;
  --tw-shadow: 0 0 #0000;
  --tw-shadow-colored: 0 0 #0000;

test/dummy/config/puma.rb (44 lines)

# frozen_string_literal: true

# This configuration file will be evaluated by Puma. The top-level methods that
# are invoked here are part of Puma's configuration DSL. For more information
# about methods provided by the DSL, see https://puma.io/puma/Puma/DSL.html.
#
# Puma starts a configurable number of processes (workers) and each process
# serves each request in a thread from an internal thread pool.
#
# You can control the number of workers using ENV["WEB_CONCURRENCY"]. You
# should only set this value when you want to run 2 or more workers. The
# default is already 1.
#
# The ideal number of threads per worker depends both on how much time the
# application spends waiting for IO operations and on how much you wish to
# prioritize throughput over latency.
#
# As a rule of thumb, increasing the number of threads will increase how much
# traffic a given process can handle (throughput), but due to CRuby's
# Global VM Lock (GVL) it has diminishing returns and will degrade the
# response time (latency) of the application.
#
# The default is set to 3 threads as it's deemed a decent compromise between
# throughput and latency for the average Rails application.
#
# Any libraries that use a connection pool or another resource pool should
# be configured to provide at least as many connections as the number of
# threads. This includes Active Record's `pool` parameter in `database.yml`.
threads_count = ENV.fetch("RAILS_MAX_THREADS", 3)
threads threads_count, threads_count

# Specifies the `port` that Puma will listen on to receive requests; default is 3000.
port ENV.fetch("PORT", 3000)

# Allow puma to be restarted by `bin/rails restart` command.
plugin :tmp_restart

# Specify the PID file. Defaults to tmp/pids/server.pid in development.
# In other environments, only set the PID file if requested.
pidfile ENV["PIDFILE"] if ENV["PIDFILE"]

# Enable YJIT if available (Ruby 3.3+ / 4.0+)
# Must be done at boot, not in before_fork (which only runs in cluster mode).
RubyVM::YJIT.enable if defined?(RubyVM::YJIT) && RubyVM::YJIT.respond_to?(:enable)

test/dummy/db/schema.rb (449 lines, first 30 shown)

# This file is auto-generated from the current state of the database. Instead
# of editing this file, please use the migrations feature of Active Record to
# incrementally modify your database, and then regenerate this schema definition.
#
# This file is the source Rails uses to define your schema when running `bin/rails
# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to
# be faster and is potentially less error prone than running all of your
# migrations from scratch. Old migrations may fail to apply correctly if those
# migrations use external dependencies or application code.
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[8.1].define(version: 2026_03_13_120000) do
  # These are extensions that must be enabled in order to support this database
  enable_extension "pg_catalog.plpgsql"

  create_table "active_storage_attachments", force: :cascade do |t|
    t.bigint "blob_id", null: false
    t.datetime "created_at", null: false
    t.string "name", null: false
    t.bigint "record_id", null: false
    t.string "record_type", null: false
    t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id"
    t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true
  end

  create_table "active_storage_blobs", force: :cascade do |t|
    t.bigint "byte_size", null: false
    t.string "checksum"
    t.string "content_type"

test/dummy/Gemfile.lock (415 lines, first 30 shown)

PATH
  remote: ../..
  specs:
    source_monitor (0.11.1)
      cssbundling-rails (~> 1.4)
      faraday (~> 2.9)
      faraday-follow_redirects (~> 0.4)
      faraday-gzip (~> 3.0)
      faraday-retry (~> 2.2)
      feedjira (>= 3.2, < 5.0)
      jsbundling-rails (~> 1.3)
      nokolexbor (~> 0.5)
      rails (>= 8.0.3, < 10.0)
      ransack (~> 4.2)
      ruby-readability (~> 0.7)
      solid_cable (>= 3.0, < 4.0)
      solid_queue (>= 0.3, < 3.0)
      turbo-rails (~> 2.0)
      view_component (>= 3.0, < 4.0)

GEM
  remote: https://rubygems.org/
  specs:
    action_text-trix (2.1.16)
      railties
    actioncable (8.1.2)
      actionpack (= 8.1.2)
      activesupport (= 8.1.2)
      nio4r (~> 2.0)
      websocket-driver (>= 0.6.1)

Active Plan

phase: 7 plan: 01 title: Model Correctness & Data Integrity type: execute wave: 1 depends_on: [] cross_phase_deps: [] autonomous: true effort_override: thorough skills_used: [sm-domain-model, sm-engine-test, tdd-cycle] files_modified:

• app/jobs/source_monitor/log_cleanup_job.rb
• app/models/source_monitor/source.rb
• app/models/source_monitor/item.rb
• app/models/source_monitor/item_content.rb
• app/models/source_monitor/import_history.rb
• app/models/concerns/source_monitor/loggable.rb
• app/models/source_monitor/fetch_log.rb
• app/models/source_monitor/scrape_log.rb
• app/models/source_monitor/health_check_log.rb
• db/migrate/TIMESTAMP_align_health_status_default.rb
• test/jobs/source_monitor/log_cleanup_job_test.rb
• test/models/source_monitor/source_test.rb
• test/models/source_monitor/item_test.rb
• test/models/source_monitor/item_content_test.rb
• test/models/source_monitor/import_history_test.rb
• test/models/source_monitor/fetch_log_test.rb forbidden_commands: [] must_haves: truths:
  • "LogCleanupJob deletes LogEntry records before deleting FetchLog/ScrapeLog/HealthCheckLog records"
  • "Source health_status has inclusion validation matching HEALTH_STATUS_VALUES constant"
  • "DB default for health_status matches model default (migration changes DB to 'working')"
  • "sync_log_entry callback is defined in Loggable concern, not in individual log models"
  • "Item#restore! method exists and increments counter cache" artifacts:
  • {path: "app/models/concerns/source_monitor/loggable.rb", provides: "Consolidated sync_log_entry callback", contains: "sync_log_entry"}
  • {path: "app/models/source_monitor/source.rb", provides: "health_status validation + scraping scopes", contains: "HEALTH_STATUS_VALUES"}
  • {path: "app/models/source_monitor/item.rb", provides: "restore! method", contains: "def restore!"} key_links:
  • {from: "app/jobs/source_monitor/log_cleanup_job.rb", to: "app/models/source_monitor/log_entry.rb", via: "LogEntry.where(loggable_type:).delete_all before log deletion"}

Key context: LogCleanupJob uses batch.delete_all on FetchLog/ScrapeLog which skips dependent: :destroy callbacks, orphaning LogEntry records. Source has health_status default "working" in model but "healthy" in DB schema. Three log models duplicate identical sync_log_entry callbacks that belong in their shared Loggable concern. Item#soft_delete! decrements counter cache but has no restore! to re-increment. </context> <tasks> <task type="auto"> <name>Fix LogCleanupJob orphaned LogEntry records (H1)</name> <files> app/jobs/source_monitor/log_cleanup_job.rb test/jobs/source_monitor/log_cleanup_job_test.rb </files> <action> In LogCleanupJob, before each batch.delete_all call on FetchLog/ScrapeLog/HealthCheckLog, first delete the corresponding LogEntry records:

# Before deleting FetchLog batch:
SourceMonitor::LogEntry.where(loggable_type: "SourceMonitor::FetchLog", loggable_id: batch.select(:id)).delete_all
batch.delete_all

Apply this pattern to all three log type cleanup sections. Add tests that:

1. Create a FetchLog + its LogEntry
2. Run LogCleanupJob
3. Assert both FetchLog AND LogEntry are deleted (no orphans remain) </action>

Note: Check what values the health module actually uses. The model says default "working", health module may use "healthy", "declining", "improving", "failing". Include all values that appear in the codebase. </action> <verify> PARALLEL_WORKERS=1 bin/rails test test/models/source_monitor/source_test.rb </verify> <done> Source has HEALTH_STATUS_VALUES constant, inclusion validation on health_status, migration aligns DB default to "working", scraping_enabled/scraping_disabled scopes added. </done> </task> <task type="auto"> <name>Add Item#restore! and fix counter cache symmetry (M3)</name> <files> app/models/source_monitor/item.rb test/models/source_monitor/item_test.rb </files> <action>

1. Add restore! method to Item that sets deleted_at to nil via update_columns and calls Source.increment_counter(:items_count, source_id). This is the symmetric counterpart to soft_delete!.
2. Write tests: restore! clears deleted_at, increments counter cache, raises on non-deleted item (or is idempotent -- follow the pattern soft_delete! uses). </action>

Research Findings

Rails Best Practices Audit — 2026-03-14

Executive Summary

5 parallel agents audited the entire SourceMonitor engine codebase across models, controllers, services/jobs/pipeline, views/frontend, and testing layers.

Overall verdict: The codebase is well-structured for an engine of this complexity. Routes follow CRUD conventions, concerns are well-scoped, and the Hotwire frontend has solid fundamentals. Recent milestone work (phases 01-06) already resolved ~19 findings. The remaining gaps are primarily business logic in jobs (violates "shallow jobs" convention) and duplicated logic across pipeline layers.

Note: 19 findings from the initial 63 were already addressed by commits in the recent ui-fixes-and-smart-scraping milestone. These are marked strikethrough below. The counts above reflect only unresolved findings.

Table of Contents

• HIGH Severity Findings
• MEDIUM Severity Findings
• LOW Severity Findings
• Top 10 Actions
• Positive Observations

HIGH Severity Findings

H1. LogCleanupJob orphans LogEntry records

• File(s): app/jobs/source_monitor/log_cleanup_job.rb:42-49
• Current: Uses batch.delete_all on FetchLog/ScrapeLog records. These models have has_one :log_entry, dependent: :destroy, but delete_all skips callbacks, orphaning LogEntry records.
• Recommended: Delete LogEntry records first by loggable_type/loggable_id, then delete the log records. Or use destroy_in_batches.
• Rationale: Orphaned LogEntry records accumulate over time, consuming disk space and corrupting the unified logs view. The dependent: :destroy declaration shows cascade was intended.
• Effort: short

H2. ImportOpmlJob contains 160 lines of business logic

• File(s): app/jobs/source_monitor/import_opml_job.rb:14-157
• Current: The job contains entry selection, deduplication, source creation, attribute building, broadcast logic, and error aggregation. This is multi-model orchestration (Source, ImportHistory, ImportSession) in a job.
• Recommended: Extract to SourceMonitor::ImportSessions::OPMLImporter service. The job becomes a 5-line delegation.
• Rationale: Violates "shallow jobs: only deserialization + delegation." Import logic cannot be invoked synchronously (console, tests) without going through ActiveJob. Spans 3+ models, qualifying for a service object.
• Effort: medium

H3. ScrapeItemJob contains rate-limiting, state management, and deferral logic

• File(s): app/jobs/source_monitor/scrape_item_job.rb:14-57
• Current: Checks scraping-enabled status, computes time-until-scrape-allowed, manages state transitions (mark_processing!, mark_failed!, clear_inflight!), and re-enqueues itself with a delay.
• Recommended: Move pre-flight checks and state management into Scraping::Runner. Job becomes a one-liner delegation.
• Rationale: Rate-limiting in time_until_scrape_allowed duplicates near-identical logic in Scraping::Enqueuer#time_rate_limited?. Two places to maintain the same business rule.
• Effort: medium

H4. DownloadContentImagesJob contains multi-model orchestration

• File(s): app/jobs/source_monitor/download_content_images_job.rb:17-49
• Current: Builds ItemContent, downloads images, creates ActiveStorage blobs, rewrites HTML, and updates the item.
• Recommended: Extract to SourceMonitor::Images::Processor. Job delegates with a single call.
• Rationale: Multi-model orchestration (Item, ItemContent, ActiveStorage::Blob) belongs in a pipeline class, not a job.
• Effort: short

H5. Scrape rate-limiting duplicated in two places

• File(s): app/jobs/source_monitor/scrape_item_job.rb:47-57 and lib/source_monitor/scraping/enqueuer.rb:129-143
• Current: Both compute time since last scrape vs. min_scrape_interval. The Enqueuer defers the job, and the Job re-checks and defers again.
• Recommended: Remove the check from ScrapeItemJob. The Enqueuer already handles deferral at enqueue time. If race conditions are a concern, have the job delegate to a runner that calls the Enqueuer.
• Rationale: Redundant DB queries and divergence risk if one is updated without the other.
• Effort: quick

H6. Source.destroy_all in pagination tests is not parallel-safe

• File(s): test/controllers/source_monitor/sources_controller_test.rb:252,264,276,286,297,311,322
• Current: Seven tests call Source.destroy_all to get a clean slate for pagination counting. With thread-based parallelism, this can race with other threads.
• Recommended: Scope assertions to test-created records using a naming pattern or tracking IDs. Or use a dedicated test class with proper isolation.
• Rationale: Violates the project's own documented isolation rule in TEST_CONVENTIONS.md section 6.
• Effort: medium

MEDIUM Severity Findings

Models & Concerns

M1. health_status default mismatch between model and database

• File(s): app/models/source_monitor/source.rb:37 vs db/schema.rb:384
• Current: Model declares attribute :health_status, :string, default: "working" but schema has default: "healthy". A Source created in Ruby gets "working", one via raw SQL gets "healthy".
• Recommended: Align the defaults. Add validates :health_status, inclusion: { in: HEALTH_STATUS_VALUES }.
• Effort: quick

M2. Missing health_status validation

• File(s): app/models/source_monitor/source.rb:44-51
• Current: fetch_status has an inclusion validation; health_status has none. Any arbitrary string can be stored.
• Recommended: Add HEALTH_STATUS_VALUES = %w[healthy working declining failing].freeze and validates :health_status, inclusion: { in: HEALTH_STATUS_VALUES }.
• Effort: quick

M3. Item#soft_delete! counter cache fragility

• File(s): app/models/source_monitor/item.rb:69-83
• Current: Manually calls Source.decrement_counter(:items_count, source_id) after update_columns. No corresponding restore! method to re-increment.
• Recommended: Add a restore! method for symmetry. Consider extracting soft-delete into a concern.
• Effort: short

M4. Duplicated sync_log_entry callback across 3 log models

• File(s): app/models/source_monitor/fetch_log.rb:28, scrape_log.rb:20, health_check_log.rb:20
• Current: All three define identical after_save :sync_log_entry callbacks.
• Recommended: Move into the Loggable concern.
• Effort: quick

Controllers & Routes

M5. No rescue_from ActiveRecord::RecordNotFound

• File(s): app/controllers/source_monitor/application_controller.rb
• Current: No rescue_from handlers. A missing record raises 500 in production.
• Recommended: Add a Turbo-aware RecordNotFound handler that renders a toast + 404.
• Rationale: As a mountable engine, SourceMonitor should handle its own common exceptions gracefully.
• Effort: short

M6. Duplicated set_source across 7 controllers

• File(s): source_fetches_controller.rb, source_retries_controller.rb, source_bulk_scrapes_controller.rb, source_health_checks_controller.rb, source_health_resets_controller.rb, source_favicon_fetches_controller.rb, source_scrape_tests_controller.rb
• Current: Each defines identical def set_source; @source = Source.find(params[:source_id]); end.
• Recommended: Extract to a SetSource concern.
• Effort: quick

M7. fallback_user_id creates users in host-app tables

• File(s): app/controllers/source_monitor/import_sessions_controller.rb:244-276
• Current: When no authenticated user exists, creates a "guest" user by introspecting column schema.
• Recommended: Guard behind Rails.env.development? or remove entirely. An engine should never create records in host-app tables.
• Effort: short

M8. ImportSessions controller concerns contain significant business logic

• File(s): app/controllers/source_monitor/import_sessions/opml_parser.rb (128 lines), entry_annotation.rb (187 lines), health_check_management.rb (112 lines), bulk_configuration.rb (106 lines)
• Current: XML parsing, URL validation, duplicate detection, job enqueueing, database locking — all in controller concerns.
• Recommended: Extract pure-domain parts to lib/ or app/services/ classes. Controller concerns become thin wrappers.
• Effort: large

M9. SourcesController#index has 47 lines of query orchestration RESOLVED

• Resolved by a6d7148 (extract sources index metrics) + 795b7b8 (SourcesFilterPresenter) + cafefc2 (FilterDropdownComponent)

M10. BulkScrapeEnablementsController contains business logic

• File(s): app/controllers/source_monitor/bulk_scrape_enablements_controller.rb:13-19
• Current: update_all with field combination for enabling scraping is in the controller.
• Recommended: Extract to Source.enable_scraping!(ids) class method.
• Effort: quick

M11. Excessive update_column usage in ImportSessions flow

• File(s): app/controllers/source_monitor/import_sessions_controller.rb (11 calls)
• Current: Skips validations for current_step and selected_source_ids changes.
• Recommended: Encapsulate in model methods like ImportSession#advance_to!(step).
• Effort: short

Services, Jobs & Pipeline

M12. FaviconFetchJob contains cooldown and attachment logic

• File(s): app/jobs/source_monitor/favicon_fetch_job.rb:17-42
• Current: Cooldown checking duplicated with SourceUpdater#enqueue_favicon_fetch_if_needed.
• Recommended: Extract to Favicons::FetchService. Consolidate cooldown in Favicons::CooldownCheck.
• Effort: short

M13. ImportSessionHealthCheckJob contains lock management

• File(s): app/jobs/source_monitor/import_session_health_check_job.rb:18-63
• Current: Acquires row lock, merges results, updates state, broadcasts.
• Recommended: Extract to ImportSessions::HealthCheckUpdater.
• Effort: short

M14. SourceHealthCheckJob contains broadcast and toast formatting

• File(s): app/jobs/source_monitor/source_health_check_job.rb:29-83
• Current: toast_payload, broadcast_outcome, trigger_fetch_if_degraded are all presentation/side-effect logic.
• Recommended: Move into Health::SourceHealthCheckOrchestrator.
• Effort: short

M15. Inconsistent Result pattern across pipeline classes PARTIALLY RESOLVED

• e03723d added Result structs to completion handlers; 5bd538a wired Result usage in FetchRunner
• Remaining: FeedFetcher::Result still lacks success?. No shared base SourceMonitor::Result class exists yet.
• Effort: medium

M16. Retry logic split across 4 locations RESOLVED

• Resolved by 4ff8884 (extract FetchFeedJob retry orchestrator service)

M17. Swallowed exceptions in ensure/rescue blocks

• File(s): feed_fetcher.rb:331, scraping/state.rb:68, source_health_check_job.rb:46-47
• Current: rescue StandardError => nil silently swallows failures.
• Recommended: Add Rails.logger.warn in rescue blocks.
• Effort: quick

M18. StalledFetchReconciler uses fragile PG JSON operator on SolidQueue internals

• File(s): lib/source_monitor/fetching/stalled_fetch_reconciler.rb:107
• Current: where("arguments::jsonb -> 'arguments' ->> 0 = ?", source.id.to_s) — fragile if SolidQueue changes serialization.
• Recommended: Add version comment and regression test.
• Effort: quick

Views & Frontend

M19. Database queries executed in view templates

• File(s): items/_details.html.erb:6-7, sources/_bulk_scrape_modal.html.erb:3-11
• Current: item.scrape_logs.order(...).limit(5) and similar queries directly in ERB.
• Recommended: Move to controllers and pass as locals, or extend presenters.
• Note: sources/_details.html.erb was partially addressed by SourceDetailsPresenter (7c2604b), but items and bulk scrape modal still have inline queries.
• Effort: short

M20. StatusBadge markup duplicated 12+ times

• File(s): sources/_row.html.erb, sources/_details.html.erb, dashboard/_recent_activity.html.erb, items/_details.html.erb, items/index.html.erb, logs/index.html.erb
• Current: Hand-crafted <span class="inline-flex items-center rounded-full ..."> with conditional spinners.
• Recommended: Create a StatusBadgeComponent.
• Note: IconComponent was added (caa4e69) for SVG icons, but status badges are a separate pattern that still needs extraction.
• Effort: medium

M21. Missing SourceDetailsPresenter PARTIALLY RESOLVED

• SourceDetailsPresenter added in 7c2604b
• Remaining: ItemDetailsPresenter, FetchLogPresenter, ScrapeLogPresenter, SourceRowPresenter still missing.
• Effort: medium (each)

M22. Modal missing role="dialog" and aria-modal

• File(s): sources/_bulk_scrape_modal.html.erb, sources/_bulk_scrape_enable_modal.html.erb
• Current: Modal panels are plain <div> elements.
• Recommended: Add role="dialog", aria-modal="true", aria-labelledby pointing to heading.
• Effort: quick

M23. Modal controller lacks focus trapping

• File(s): app/assets/javascripts/source_monitor/controllers/modal_controller.js
• Current: Handles Escape and backdrop click but doesn't trap focus. Users can Tab out.
• Recommended: Implement focus trapping with inert attribute on background elements.
• Rationale: WCAG 2.1 SC 2.4.3 requires meaningful focus order.
• Effort: medium

M24. Logs index missing Turbo Frame for filter/pagination

• File(s): app/views/source_monitor/logs/index.html.erb
• Current: No Turbo Frame wrapping. form_with uses local: true disabling Turbo. Full page reload on filter.
• Recommended: Wrap table+pagination in a Turbo Frame. Sources and Items both use this pattern.
• Effort: medium

M25. Button styles inconsistent across templates

• File(s): Virtually every template
• Current: 5-6 button variants with inconsistent font-semibold vs font-medium, padding variations.
• Recommended: Extract button variants to @apply CSS classes or a ButtonComponent.
• Effort: medium

M26. ApplicationHelper is 333 lines with mixed concerns

• File(s): app/helpers/source_monitor/application_helper.rb
• Current: 20+ methods spanning badges, favicons, pagination, URLs, formatting.
• Recommended: Split into focused helper modules: StatusBadgeHelper, FaviconHelper, PaginationHelper, FetchIntervalHelper, ExternalLinkHelper.
• Note: SourcesFilterPresenter (795b7b8) and FilterDropdownComponent (cafefc2) extracted some filter logic, but the helper itself is still large.
• Effort: medium

Testing

M27. create_item! factory underused PARTIALLY RESOLVED

• 18692f6 centralized factory helpers into ModelFactories module
• Remaining: Many test files still use manual Item.create! instead of the now-centralized create_item!. Migration to the shared factories is incomplete.
• Effort: medium

LOW Severity Findings

Controllers & Routes

L1. new action delegates to create in ImportSessionsController (GET creates records)

• import_sessions_controller.rb:35-37 — quick

L2. BulkScrapeEnablementsController accesses params without strong params wrapper

• bulk_scrape_enablements_controller.rb:6 — quick

L3. SourceScrapeTestsController#create builds result hash inline (should be presenter)

• source_scrape_tests_controller.rb:14-24 — short

L4. SourceHealthChecksController embeds Tailwind classes in controller

• source_health_checks_controller.rb:25-31 — quick

L5. Inconsistent Turbo Stream response patterns (StreamResponder vs raw arrays)

• Multiple controllers — short

L6. Broad rescue StandardError in action controllers RESOLVED

• Resolved by 6bcd0ac and 19bb3b8 (transient vs fatal error classification in FaviconFetchJob and DownloadContentImagesJob) + 911c17e (deadlock rescue + error logging)

L7. SanitizesSearchParams uses to_unsafe_h without documentation

• concerns/source_monitor/sanitizes_search_params.rb:45 — quick

L8. SourcesController#update contains conditional job-enqueue logic

• sources_controller.rb:94-109 — short

Models

L9. Ransacker subqueries could be extracted to Source::SearchableAttributes concern

• source.rb:79-103 — short

L10. Missing scraping_enabled / scraping_disabled scopes

• source.rb — quick

L11. ItemContent#compute_feed_word_count reaches through association (minor Demeter violation)

• item_content.rb:33-39 — quick

L12. ImportHistory missing chronological validation and JSONB attribute declarations

• import_history.rb — quick

Services, Jobs & Pipeline

L13. FetchFeedJob#should_run? scheduling logic PARTIALLY RESOLVED

• 4ff8884 extracted retry orchestrator, reducing job logic. should_run? guard still exists but is simpler.

L14. Backward-compatibility forwarding methods in FeedFetcher (12) and ItemCreator (18)

• feed_fetcher.rb:393-404, item_creator.rb:179-197 — medium

L15. FeedFetcher constants duplicated from AdaptiveInterval

• feed_fetcher.rb:30-36 — quick

L16. Inconsistent logger guard pattern (20+ occurrences of full guard)

• Nearly every pipeline file — short

L17. Images::Downloader creates raw Faraday connection instead of HTTP.client

• images/downloader.rb:46-58 — quick

L18. Logs::Query is good; Scheduler queries could be extracted

• scheduler.rb:55-89, scraping/scheduler.rb:38-45 — short

L19. CloudflareBypass tries all 4 user agents sequentially (could cause 60s+ fetch)

• fetching/cloudflare_bypass.rb:39-49 — quick

Views & Frontend

L20. Items and Logs index pagination not using shared _pagination.html.erb partial

• items/index.html.erb:124-146, logs/index.html.erb:186-209 — short

L21. Scrape test result markup duplicated between show page and modal

• source_scrape_tests/show.html.erb:8-56, _result.html.erb:12-60 — quick

L22. Card panel pattern repeated ~20 times (potential PanelComponent)

• Various dashboard/sources/items views — medium

L23. Error display duplicated across new/edit views (should be _form_errors partial)

• sources/new.html.erb:4-13, edit.html.erb:4-13 — quick

L24. Dropdown controller registers global click listener eagerly RESOLVED

• Resolved by 491fae1 (simplify dropdown controller and remove JS globals)

L25. Notification controller has dead applyLevelDelay() method RESOLVED

• Resolved by 15e7d53 (remove dead JS error delay override and document constants)

L26. Dismiss button SVG missing aria-label, should use IconComponent PARTIALLY RESOLVED

• 4c56789 replaced inline SVGs with IconComponent. Check if this specific dismiss button was included.

L27. FilterDropdownComponent uses inline onchange instead of Stimulus action

• filter_dropdown_component.rb:48,55 — short

Testing

L28. Duplicated configure_authentication helper across 4 test files

• 4 test files — quick

L29. Duplicated SolidQueue table purge logic PARTIALLY RESOLVED

• 54617b8 created SystemTestHelpers module with shared purge method. Some lib tests may still inline it.

L30. No test files for FetchLogsController, ScrapeLogsController, ImportHistory model

• Missing test files — short

Top 10 Actions (prioritized by impact/effort ratio)

Previously in Top 10, now resolved: M9 (SourcesController#index metrics — extracted to presenters), M16 (retry logic consolidation — extracted to RetryOrchestrator service)

Positive Observations

The audit identified many areas where the codebase excels:

• CRUD route design is textbook — every action is a resource (source_fetches, source_retries, etc.)
• Loggable concern is exemplary single-purpose shared behavior
• Boolean usage correctly follows state-as-records convention (all booleans are technical flags)
• Association declarations include inverse_of, dependent: :destroy, and thorough indexing
• Factory helpers (ModelFactories) have good defaults with SecureRandom for parallel safety
• VCR/WebMock separation is clean (VCR for real feeds, WebMock for controlled scenarios)
• Source turbo responses concern is well-focused on response rendering
• Table styling is consistent across all views
• Test conventions are documented in TEST_CONVENTIONS.md with clear guidance
• Thread-safe config reset with SourceMonitor.reset_configuration!
• Active Storage guarding with if defined?(ActiveStorage) checks
• Strong params via Sources::Params.sanitize with explicit allowlist
• Pipeline architecture — all service objects are justified multi-model orchestrators

Generated by /rails-audit command. Re-run to refresh findings.