Skip to content

Conversation

@fdematos
Copy link
Contributor

Summary

  • Add SQLite-backed ROTI feedback API with validation and average computation
  • Add backend unit tests for ROTI business rules
  • Build a Tailwind-based ROTI voting SPA with live stats and feedback list

@github-actions
Copy link

APIError:

opencode session  |  github run

@github-actions
Copy link

Code Review: feat/roti-live

Summary

This is a well-structured PR adding a ROTI (Return On Time Invested) feedback feature. The code follows the project's conventions and has good separation of concerns. Here are my findings:

Issues Found

1. Missing @types/better-sqlite3
apps/backend/package.json - The backend uses better-sqlite3 but lacks @types/better-sqlite3 in devDependencies. This could cause TypeScript issues.

2. Database connection never closed
apps/backend/src/roti-db.ts:13 - The database connection is created but never closed. This can cause issues during testing or server shutdown.

3. No maximum comment length validation
apps/backend/src/roti-service.ts - Comments have no length limit. Consider adding a max length (e.g., 1000 chars) to prevent storage abuse and UI issues.

4. Fragile date formatting
apps/frontend/src/App.tsx:194 - Using slice(11, 16) on ISO date strings is fragile. Consider using Intl.DateTimeFormat or a date library for proper timezone handling.

5. Hardcoded rating scale
apps/frontend/src/App.tsx:16 - The ratings array [1, 2, 3, 4, 5] is hardcoded but the backend validation in roti-service.ts:16 also defines the 1-5 range. These could drift. Consider sharing a constant.

6. Database singleton makes testing difficult
apps/backend/src/roti-db.ts:13 - The exported db is a singleton instance. This makes it hard to reset state between tests or mock the database.

Suggestions

1. Add graceful shutdown
Add process exit handlers to close the database connection:

process.on('SIGINT', () => {
  db.close();
  process.exit(0);
});

2. Display "N/A" for empty averages
apps/frontend/src/App.tsx:173 - When data.count === 0, showing "0.0" as the average is misleading. Consider showing "-" or "No votes yet".

3. Add API integration tests
The tests only cover the service layer. Consider adding tests for the actual API endpoints in index.test.ts.

4. Extract shared constants
Create a shared constants file for the rating scale (1-5) used in both frontend and backend.

Positive Points

  • Clean separation of concerns (db layer, service layer, routes)
  • Good TypeScript interfaces and type safety
  • Proper error handling with appropriate HTTP status codes
  • Good test coverage for business logic
  • Client-side validation matches server-side rules
  • Clean React component with proper loading/error states
  • Follows AGENTS.md conventions (imports order, naming, strict mode)

Verdict

Approve with minor suggestions. The code is production-ready but would benefit from the suggestions above, especially adding @types/better-sqlite3 and closing database connections properly.

New%20session%20-%202026-01-28T16%3A16%3A17.964Z
opencode session  |  github run

@fdematos fdematos merged commit b11bedd into main Jan 28, 2026
2 of 3 checks passed
@fdematos fdematos deleted the feat/roti-live branch January 28, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants