Skip to content

Conversation

@fdematos
Copy link
Contributor

Summary

  • add SQLite-backed ROTI feedback API with validation and unit tests
  • build Tailwind-based SPA UI for ratings, average display, and feedback list
  • update Dockerfile permissions and ignore runtime data directory

@github-actions
Copy link

Code Review Summary

Overall, this is a well-structured PR that adds ROTI feedback functionality with good separation of concerns and proper validation. However, there are several issues to address before merging.

Issues Found

Language Consistency (AGENTS.md violation)

  • Error messages in roti-service.ts:20,25 are in French
  • UI text in App.tsx is in French
  • Per project guidelines, all code, comments, and messages should be in English

Backend Issues

  • roti-db.ts:13 - Database connection is created at module load time (side effect), making testing difficult
  • roti-db.ts - No cleanup/shutdown hook to close database connections (resource leak risk)
  • index.ts:46 - Number(req.body?.rating) can produce non-integer values (e.g., 3.7) which would pass validation but fail DB constraint
  • index.test.ts - Only unit tests for service functions; missing integration tests for API endpoints

Frontend Issues

  • App.tsx:43,81 - Type assertions (as RotiResponse, as Error) bypass TypeScript safety
  • App.tsx:47,90 - Error casting with (err as Error) is unsafe; use proper error handling
  • App.tsx:36 - canSubmit logic is confusing; could be simplified

Suggestions

  1. Internationalization: Move all user-facing strings to English to follow project conventions
  2. Database Lifecycle: Add graceful shutdown handling to close DB connections
  3. Type Safety: Replace type assertions with runtime validation (e.g., Zod)
  4. Testing: Add API integration tests for the new endpoints
  5. Validation: Use Number.isInteger() after conversion in the POST handler

Positive Aspects

  • Good separation of concerns (service layer, database layer)
  • Proper input validation with clear error messages
  • Clean UI with Tailwind and good UX (loading states, disabled submit)
  • Database constraints match application validation
  • Dockerfile properly sets up data directory permissions
  • .gitignore correctly ignores runtime data

The PR is conditionally approved pending the language consistency fixes and minor improvements noted above.

New%20session%20-%202026-01-28T17%3A00%3A45.068Z
opencode session  |  github run

@fdematos fdematos merged commit 1f1b4d7 into main Jan 28, 2026
3 checks passed
@fdematos fdematos deleted the feature/roti-feedback branch January 28, 2026 17:09
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