# Codebase Concerns

**Analysis Date:** 2026-01-11

## Tech Debt

**God Objects - Massive Files:**
- Issue: Several models and helpers have grown too large, violating single responsibility
- Files:
  - `ruby/lib/jam_ruby/jam_track_importer.rb` - 3,617 lines (import logic)
  - `ruby/lib/jam_ruby/models/user.rb` - 3,078 lines (user model with too many responsibilities)
  - `ruby/lib/jam_ruby/models/notification.rb` - 1,700 lines
  - `ruby/lib/jam_ruby/models/music_session.rb` - 1,481 lines
  - `ruby/lib/jam_ruby/models/lesson_session.rb` - 1,312 lines
  - `web/app/controllers/api_users_controller.rb` - 1,407 lines
  - `jam-ui/src/helpers/MixerHelper.js` - 1,270 lines (complex audio mixer logic)
  - `jam-ui/src/hooks/useMixerHelper_delete.js` - 1,326 lines (appears to be dead code)
  - `web/app/assets/javascripts/session.js` - 3,345 lines
  - `web/app/assets/javascripts/jam_rest.js` - 3,227 lines
- Impact: Hard to maintain, test, understand; high coupling
- Fix approach: Break into smaller modules by responsibility, extract services/managers

**N+1 Query Problems:**
- Issue: Commented-out eager loading causing N+1 queries
- Files:
  - `ruby/lib/jam_ruby/models/jam_track.rb:387` - "FIXME: n+1 queries for rights and genres"
  - `ruby/lib/jam_ruby/models/feed.rb:35` - "TODO: SPEED UP QUERY. CURRENTLY TAKES FOR EVER."
- Impact: Performance degradation under load, slow API responses
- Fix approach: Uncomment and fix eager loading with `includes()`, add database indexes

**Synchronous Operations (Should Be Async):**
- Issue: Blocking operations that should run in background jobs
- Files:
  - `ruby/lib/jam_ruby/models/notification.rb:82-83` - All notification methods should be async
  - `ruby/lib/jam_ruby/models/user.rb:1379` - "TODO: make this async" (Notification.send_new_user_follower)
  - `ruby/lib/jam_ruby/models/user.rb:1392` - "TODO: make this async" (Notification.send_new_band_follower)
  - `ruby/lib/jam_ruby/models/notification.rb:543` - "TODO: do this in BULK or in async block"
- Impact: Slow request/response times, poor user experience
- Fix approach: Move to Resque background jobs, use AMQP for async messaging

**Inefficient In-Memory Operations:**
- Issue: Loading large datasets into memory instead of counting in database
- Files:
  - `ruby/lib/jam_ruby/models/band.rb:74` - "FIXME: this could be a lot of followers; calling size loads all the data into memory"
  - Multiple methods in `ruby/lib/jam_ruby/models/user.rb` using `.size` instead of `.count()`
- Impact: High memory usage, potential OOM errors with large datasets
- Fix approach: Use `.count()` for database counting, avoid loading associations into memory

**Dead/Duplicate Code:**
- Issue: Legacy files not cleaned up
- Files:
  - `jam-ui/src/hooks/useMixerHelper_delete.js` - 1,326 lines (appears to be backup)
  - `jam-ui/src/sessions/JKLobbyChat copy.js` - Duplicate/backup file
- Impact: Code confusion, increased maintenance burden
- Fix approach: Delete dead code, ensure backups via Git history

## Known Bugs

**Race Conditions:**
- Symptoms: Multiple users completing uploads simultaneously triggers duplicate jobs
- Trigger: Two users complete recording uploads at same time
- File: `ruby/lib/jam_ruby/models/recording.rb:685` - "FIXME: There's a possible race condition here. If two users complete uploads at the same time, we'll schedule 2 mixes."
- Root cause: No atomic check-and-set for upload completion
- Fix: Add database-level locking or use Redis distributed lock

**Timing Vulnerabilities:**
- Symptoms: Lesson session scheduling can create past sessions
- Trigger: Scheduling logic with week offset
- File: `ruby/lib/jam_ruby/models/lesson_session.rb:1040` - "XXX: week offset as 0? This *could* still be in the past."
- Root cause: Time calculation doesn't account for current moment
- Fix: Validate scheduled time is always in future before saving

## Security Considerations

**SQL Injection Vulnerabilities:**
- Risk: String interpolation in SQL queries allows injection attacks
- Files:
  - `ruby/lib/jam_ruby/models/notification.rb:75` - `Notification.delete_all "(session_id = '#{self.session_id}')"`
  - `ruby/lib/jam_ruby/models/join_request.rb:25` - `JoinRequest.where("join_requests.user_id = '#{current_user.id}'")`
  - `ruby/lib/jam_ruby/models/user.rb:772` - Multiple string interpolations in where clauses
  - `ruby/lib/jam_ruby/models/user.rb:1397,1419,1469` - String interpolation in delete_all queries
  - `ruby/lib/jam_ruby/models/ip_blacklist.rb:11,15` - SQL injection in IP validation
- Current mitigation: None (direct string interpolation used)
- Recommendations: Use parameterized queries, ActiveRecord placeholders (?, named parameters)

**Hardcoded Credentials in Source:**
- Risk: Credentials exposed in version control
- Files:
  - `web/Gemfile:17` - `source 'https://jamjam:blueberryjam@int.jamkazam.com/gems/'`
  - `ruby/Gemfile:4` - Same hardcoded credentials
- Current mitigation: Private repository (still not secure)
- Recommendations: Use environment variables, secure credential storage

**HTTP Over HTTPS:**
- Risk: Unencrypted communication exposes data
- File: `ruby/lib/jam_ruby/lib/s3_manager.rb:32` - "the client can not support HTTPS atm!!! AGH"
- Current mitigation: None (explicitly using HTTP)
- Recommendations: Update client to support HTTPS, enforce SSL/TLS

**Missing Error Handling:**
- Risk: Unhandled failures expose system internals
- Files:
  - `web/lib/google_client.rb:56` - "XXX: what to do when this fails?"
  - `web/lib/google_client.rb:372` - "TODO: how to test for this:"
- Current mitigation: None
- Recommendations: Add proper error handling with user-friendly messages

## Performance Bottlenecks

**Slow Queries:**
- Problem: Feed query takes excessive time
- File: `ruby/lib/jam_ruby/models/feed.rb:35` - Marked "CURRENTLY TAKES FOR EVER."
- Measurement: Not quantified (needs profiling)
- Cause: Likely missing indexes, complex joins, or N+1 queries
- Improvement path: Profile query, add indexes, optimize joins, eager load associations

**Memory Leaks:**
- Problem: AMQP connection manager documented as "leaky"
- File: `ruby/lib/jam_ruby/amqp/amqp_connection_manager.rb:3` - "It's 'leaky' in that it will give you a AMQP::Channel"
- Measurement: Not quantified
- Cause: Channels not properly closed/released
- Improvement path: Implement proper resource cleanup, use connection pooling

## Fragile Areas

**Complex State Management:**
- File: `ruby/lib/jam_ruby/models/recorded_track.rb:4`
- Why fragile: "TODO: use aasm instead of all these bool and bool_was (dirty module) checks"
- Common failures: State transitions not properly tracked, inconsistent state
- Safe modification: Migrate to AASM state machine for better transition management
- Test coverage: Unknown (needs investigation)

**WebSocket Message Handling:**
- File: `ruby/lib/jam_ruby/message_factory.rb` (42KB)
- Why fragile: Single massive file handling 100+ message types
- Common failures: New message types not properly handled, encoding/decoding errors
- Safe modification: Extract message handlers into separate classes per message type
- Test coverage: Partial (needs more coverage for edge cases)

## Scaling Limits

**Connection Pooling:**
- Current capacity: Default PostgreSQL connection pool (5 connections per process)
- Limit: ~50 concurrent users per web/admin instance before connection exhaustion
- Symptoms at limit: ActiveRecord::ConnectionTimeoutError, slow responses
- Scaling path: Increase connection pool size, add connection pooling middleware (PgBouncer)

**File Storage:**
- Current capacity: AWS S3 (effectively unlimited)
- Limit: Cost-based rather than technical
- Symptoms at limit: Increased AWS bills
- Scaling path: Implement CDN caching, compress media files, archive old content

## Dependencies at Risk

**Ruby Version (Critical):**
- Risk: Ruby 2.3.1-2.4.1 is severely outdated (current is 3.3+), EOL, security vulnerabilities
- Impact: Missing security patches, performance improvements, language features
- Migration plan: Upgrade to Ruby 3.x (requires extensive testing, gem compatibility checks)

**Rails Version (Critical):**
- Risk: Rails 4.2.8 is EOL (current is 7.x), no security patches
- Impact: Security vulnerabilities, missing features, incompatible with modern gems
- Migration plan: Upgrade to Rails 7.x (major breaking changes, extensive refactoring required)

**React Version (Medium):**
- Risk: React 16.13.1 is two major versions behind (current is 18.x)
- Impact: Missing concurrent rendering, automatic batching, newer React features
- Migration plan: Upgrade to React 18 (relatively straightforward, test for breaking changes)

**Nokogiri (Critical):**
- Risk: nokogiri 1.10.10 has known security vulnerabilities
- Impact: XML parsing vulnerabilities, potential XSS attacks
- Migration plan: Update to latest nokogiri version (check for breaking API changes)

**Devise (Medium):**
- Risk: devise 3.3.0 is very outdated (current is 4.x)
- Impact: Missing security improvements, authentication features
- Migration plan: Upgrade to Devise 4.x (check for breaking changes in authentication flow)

## Missing Critical Features

**Email Validation:**
- Problem: No validation of email quality in user registration
- File: `ruby/lib/jam_ruby/models/user.rb:1097` - "FIXME: Should verify that the new password meets certain quality criteria"
- Current workaround: None (accepts any password format)
- Blocks: Password security compliance, account security
- Implementation complexity: Low (add password strength validation)

**Model Validations:**
- Problem: Several validations commented out or marked as TODO
- Files:
  - `ruby/lib/jam_ruby/models/rsvp_slot.rb:22` - "TODO: validates :proficiency_level"
  - `ruby/lib/jam_ruby/models/user_asset.rb:11` - "TODO: validate asset_type"
- Current workaround: No validation (relies on application-level checks)
- Blocks: Data integrity, consistent database state
- Implementation complexity: Low (uncomment and add validations)

**Async Notification System:**
- Problem: Notifications sent synchronously, blocking request/response
- Current workaround: Users wait for notifications to send
- Blocks: Fast API response times, scalability
- Implementation complexity: Medium (migrate to Resque/Sidekiq jobs)

## Test Coverage Gaps

**Complex Business Logic:**
- What's not tested: Large portions of User model (3,078 lines), MusicSession lifecycle
- Risk: Changes break critical user flows without detection
- Priority: High
- Difficulty to test: Medium (requires extensive fixtures, complex setup)

**WebSocket Gateway:**
- What's not tested: Full Protocol Buffer message flow end-to-end
- Risk: Real-time features break silently
- Priority: High
- Difficulty to test: High (requires WebSocket test infrastructure)

**Legacy JavaScript:**
- What's not tested: Many legacy JavaScript files lack tests
- Files: `web/app/assets/javascripts/session.js` (3,345 lines), `jam_rest.js` (3,227 lines)
- Risk: Breaks in production, hard to refactor
- Priority: Medium
- Difficulty to test: High (legacy code, no clear boundaries)

## Documentation Gaps

**Complex Helpers:**
- What's undocumented: `MixerHelper.js` (1,270 lines) has minimal comments
- File: `jam-ui/src/helpers/MixerHelper.js`
- Risk: Hard to understand audio mixer logic, difficult to maintain
- Priority: Medium
- Fix: Add inline documentation explaining audio concepts, mixer state management

**API Endpoints:**
- What's undocumented: No centralized API documentation for frontend consumers
- Risk: Frontend developers don't know available endpoints, parameters
- Priority: Medium
- Fix: Generate API docs from Rails routes, use Swagger/OpenAPI

**Debugging Code Left In:**
- Files:
  - `jam-ui/src/context/JamClientContext.js:42` - "TODO: Expose jamClient globally for debugging. This is temporarily added. Remove it later."
  - `jam-ui/src/helpers/MixerHelper.js:9` - "TODO - remove mixMode from MixerHelper?"
- Impact: Temporary code becomes permanent, clutters codebase
- Fix: Clean up debugging code before merging to main

---

*Concerns audit: 2026-01-11*
*Update as issues are fixed or new ones discovered*
