Refactoring

.claude/CLAUDE.md

@@ -34,8 +34,9 @@ Student Feedback Loop service for the MathTrail platform. Receives student feedb
 | `internal/config/config.go` | Config (Viper) |
 | `internal/database/dapr_binding.go` | PostgreSQL via Dapr output binding |
 | `internal/feedback/` | model, repository, service, controller |
+| `internal/apierror/apierror.go` | Shared HTTP error response type |
 | `internal/server/router.go` | Gin router with all routes & middleware |
-| `internal/server/middleware.go` | RequestID, ZapLogger, ZapRecovery, UserSpanAttributes |
+| `internal/server/middleware/` | RequestID (TraceID-linked), ZapLogger, ZapRecovery, UserSpanAttributes |
 | `internal/observability/observability.go` | InitTracer, InitMetrics, InitPyroscope |
 | `internal/clients/llm_client.go` | LLM client (future v2) |
 | `internal/clients/profile_client.go` | Profile service client |
@@ -60,7 +61,8 @@ GET    /swagger/*any         — Swagger UI
 
 ## Architecture
 
-- **DB access:** Dapr output binding (`postgres` component) — no direct pgx connection in app code
+- **DB access:** Dapr output binding (`postgres` component) via PgBouncer (`postgres-pgbouncer:6432`) — no direct pgx connection in app code
+- **Migrations:** Direct PostgreSQL (`postgres-postgresql:5432`) — bypasses PgBouncer because DDL requires session mode
 - **CDC:** Debezium monitors the `feedback` table, publishes events to Kafka — app does NOT publish events
 - **Dapr App ID:** `mentor-api`
 - Helm chart uses `mathtrail-service-lib` library chart from `https://MathTrail.github.io/charts/charts`
@@ -91,7 +93,7 @@ just load-test                 # Run k6 load tests
 - Follow Clean Architecture: Domain → Repository → Service → Controller
 - Handle errors explicitly — never ignore error returns
 - All comments in English
-- Middleware order: otelgin → UserSpanAttributes → RequestID → ZapLogger → ZapRecovery
+- Middleware order: otelgin → ZapRecovery → UserSpanAttributes → RequestID → ZapLogger
 - Commit convention: `feat(feedback):`, `fix(feedback):`, `test(feedback):`, `docs(feedback):`
 
 ## External Dependencies

.devcontainer/post-start.sh

@@ -19,6 +19,9 @@ else
     echo "Run 'just kubeconfig' in infra-local-k3s on host first"
 fi
 
+# Set up git hooks
+git config core.hooksPath .githooks
+
 # Verify cluster connection
 kubectl cluster-info 2>/dev/null && echo "Connected to cluster" \
     || echo "Cluster not accessible — check that k3d is running on host"

.githooks/pre-commit

@@ -0,0 +1,11 @@
+#!/bin/sh
+
+# Check Go formatting with gofmt -s
+UNFORMATTED=$(gofmt -s -l . 2>/dev/null)
+if [ -n "$UNFORMATTED" ]; then
+    echo "❌ Go files not formatted with gofmt -s:"
+    echo "$UNFORMATTED"
+    echo ""
+    echo "Run: just fmt"
+    exit 1
+fi

.github/workflows/ci.yml

@@ -3,6 +3,12 @@ name: CI
 on:
   pull_request:
     branches: [main]
+  workflow_dispatch:
+    inputs:
+      skip_cleanup:
+        description: "Skip namespace cleanup (for debugging)"
+        type: boolean
+        default: false
 
 permissions:
   contents: read
@@ -27,7 +33,6 @@ jobs:
           echo "VAULT_ROLE=mentor-api-role-ns-mentor-api-${{ github.run_id }}" >> "$GITHUB_ENV"
 
       - name: Prepare namespace
-        if: github.event_name == 'pull_request'
         run: just ci-prepare ${{ env.NAMESPACE }}
 
       - name: Lint
@@ -36,6 +41,13 @@ jobs:
       - name: Test
         run: just ci-test
 
+      - name: Upload coverage to Codecov
+        if: always()
+        uses: codecov/codecov-action@v5
+        with:
+          files: coverage.out
+          fail_ci_if_error: false
+
       - name: Setup
         run: just setup
 
@@ -67,6 +79,6 @@ jobs:
           path: summary.json
           retention-days: 90
 
-      # - name: Cleanup namespace
-      #   if: always() && github.event_name == 'pull_request'
-      #   run: just ci-cleanup ${{ env.NAMESPACE }}
+      - name: Cleanup namespace
+        if: always() && inputs.skip_cleanup != true
+        run: just ci-cleanup ${{ env.NAMESPACE }}

.github/workflows/docs.yml

@@ -0,0 +1,47 @@
+name: Deploy API Docs
+
+on:
+  push:
+    branches: [main]
+
+concurrency:
+  group: pages
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+  pages: write
+  id-token: write
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version: '1.25.x'
+
+      - name: Install swag
+        run: go install github.com/swaggo/swag/cmd/swag@v1.8.12
+
+      - name: Generate Swagger spec
+        run: swag init -g cmd/server/main.go -o docs/
+
+      - name: Prepare Pages artifact
+        run: cp docs/swagger.json docs/swagger-ui/
+
+      - uses: actions/upload-pages-artifact@v3
+        with:
+          path: docs/swagger-ui
+
+  deploy:
+    needs: build
+    runs-on: ubuntu-latest
+    environment:
+      name: github-pages
+      url: ${{ steps.deployment.outputs.page_url }}
+    steps:
+      - id: deployment
+        uses: actions/deploy-pages@v4

.gitignore

@@ -35,6 +35,9 @@ skaffold.env
 # .idea/
 # .vscode/
 
+# Swagger UI (CI copies swagger.json into this folder on deploy)
+docs/swagger-ui/swagger.json
+
 # esbuild output
 tests/load/dist/
 

Dockerfile

@@ -12,7 +12,6 @@ RUN apk add --no-cache ca-certificates tzdata
 RUN adduser -D -u 10001 appuser
 COPY --from=builder /app /app
 COPY --from=builder /migrate /migrate
-COPY migrations/ /migrations/
 USER 10001
 EXPOSE 8080
 ENTRYPOINT ["/app"]

README.md

@@ -1,5 +1,19 @@
 # mentor-api
 
+[![CI](https://github.com/MathTrail/mentor-api/actions/workflows/ci.yml/badge.svg)](https://github.com/MathTrail/mentor-api/actions)
+[![Go Report Card](https://goreportcard.com/badge/github.com/MathTrail/mentor-api)](https://goreportcard.com/report/github.com/MathTrail/mentor-api)
+[![codecov](https://codecov.io/gh/MathTrail/mentor-api/branch/main/graph/badge.svg)](https://codecov.io/gh/MathTrail/mentor-api)
+[![Go Version](https://img.shields.io/github/go-mod/go-version/MathTrail/mentor-api)](https://github.com/MathTrail/mentor-api/blob/main/go.mod)
+[![Go Reference](https://pkg.go.dev/badge/github.com/MathTrail/mentor-api.svg)](https://pkg.go.dev/github.com/MathTrail/mentor-api)
+
+[![Kubernetes](https://img.shields.io/badge/Kubernetes-Ready-326CE5?style=for-the-badge&logo=kubernetes)](./deploy/charts)
+[![Dapr](https://img.shields.io/badge/Dapr-Enabled-007ACC?style=for-the-badge&logo=dapr)](https://dapr.io/)
+[![PostgreSQL](https://img.shields.io/badge/PostgreSQL-16-336791?style=for-the-badge&logo=postgresql)](https://www.postgresql.org/)
+
+[![API Docs](https://img.shields.io/badge/API_Docs-Swagger-85EA2D?style=for-the-badge&logo=swagger)](https://MathTrail.github.io/mentor-api/)
+[![Tracing](https://img.shields.io/badge/Tracing-OTel-000000?style=for-the-badge&logo=opentelemetry)](https://opentelemetry.io/)
+[![Profiling](https://img.shields.io/badge/Profiling-Pyroscope-FF7800?style=for-the-badge&logo=pyroscope)](https://pyroscope.io/)
+
 Student Feedback Loop service for the MathTrail platform. Receives student feedback about task difficulty, delegates analysis to an LLM, and stores the resulting strategy in PostgreSQL.
 
 ## Mission & Responsibilities

cmd/migrate/main.go

@@ -4,82 +4,110 @@ import (
 	"database/sql"
 	"fmt"
 	"os"
-	"path/filepath"
-	"sort"
 
 	_ "github.com/jackc/pgx/v5/stdlib"
+	"github.com/pressly/goose/v3"
+	"go.uber.org/zap"
 
-	"github.com/MathTrail/mentor-api/internal/config"
 	"github.com/MathTrail/mentor-api/internal/logging"
-	"go.uber.org/zap"
+	"github.com/MathTrail/mentor-api/migrations"
 )
 
+// dbConfig holds database connection parameters read from environment variables.
+// These variables are injected only into the migration K8s Job, keeping
+// credentials out of the server binary's environment.
+type dbConfig struct {
+	Host     string
+	Port     string
+	User     string
+	Password string
+	Name     string
+	SSLMode  string
+}
+
+func loadDBConfig() dbConfig {
+	return dbConfig{
+		Host:     requiredEnv("DB_HOST"),
+		Port:     requiredEnv("DB_PORT"),
+		User:     requiredEnv("DB_USER"),
+		Password: requiredEnv("DB_PASSWORD"),
+		Name:     requiredEnv("DB_NAME"),
+		SSLMode:  envOrDefault("DB_SSL_MODE", "disable"),
+	}
+}
+
+// requiredEnv reads an environment variable or panics if it is empty.
+// A missing variable means the K8s Job is misconfigured — fail fast.
+func requiredEnv(key string) string {
+	v := os.Getenv(key)
+	if v == "" {
+		panic(fmt.Sprintf("required environment variable %s is not set", key))
+	}
+	return v
+}
+
+func envOrDefault(key, fallback string) string {
+	if v := os.Getenv(key); v != "" {
+		return v
+	}
+	return fallback
+}
+
+// dsn builds a libpq connection string for the given database name.
+func (c dbConfig) dsn(dbname string) string {
+	return fmt.Sprintf(
+		"host=%s port=%s user=%s password=%s dbname=%s sslmode=%s",
+		c.Host, c.Port, c.User, c.Password, dbname, c.SSLMode,
+	)
+}
+
 func main() {
-	// Initialize logger
-	logger := logging.NewLogger("info")
+	logger := logging.NewLogger("info", "json")
 
-	// Load configuration
-	cfg := config.Load()
+	cfg := loadDBConfig()
 
-	// Ensure the target database exists
+	// Ensure the target database exists (Goose requires it).
 	ensureDatabase(cfg, logger)
 
-	// Run SQL migrations (extensions, custom types, tables)
-	runSQLMigrations(cfg.DSN(), logger)
+	// Run Goose migrations with embedded SQL files.
+	if err := runMigrations(cfg.dsn(cfg.Name), logger); err != nil {
+		logger.Fatal("migrations failed", zap.Error(err))
+	}
 
 	logger.Info("all migrations completed successfully")
 	fmt.Println("✓ Database migrations completed successfully")
 }
 
-// runSQLMigrations executes all .sql files from the migrations directory.
-func runSQLMigrations(dsn string, logger *zap.Logger) {
-	sqlDB, err := sql.Open("pgx", dsn)
+// runMigrations applies all pending Goose migrations from the embedded FS.
+func runMigrations(dsn string, logger *zap.Logger) error {
+	db, err := sql.Open("pgx", dsn)
 	if err != nil {
-		logger.Fatal("failed to open sql connection for migrations", zap.Error(err))
+		return fmt.Errorf("failed to open db for migrations: %w", err)
 	}
-	defer func() { _ = sqlDB.Close() }()
+	// Goose applies migrations sequentially.
+	// Limit the pool to one connection for predictable migration execution.
+	db.SetMaxOpenConns(1)
+	defer func() { _ = db.Close() }()
 
-	migrationsDir := "/migrations"
-	if _, err := os.Stat(migrationsDir); os.IsNotExist(err) {
-		migrationsDir = "./migrations"
-	}
+	goose.SetBaseFS(migrations.FS)
+	goose.SetLogger(&gooseLogAdapter{l: logger})
 
-	files, err := filepath.Glob(filepath.Join(migrationsDir, "*.sql"))
-	if err != nil {
-		logger.Fatal("failed to read migrations directory", zap.Error(err))
+	if err := goose.SetDialect("postgres"); err != nil {
+		return fmt.Errorf("failed to set goose dialect: %w", err)
 	}
 
-	if len(files) == 0 {
-		logger.Info("no SQL migration files found", zap.String("directory", migrationsDir))
-		return
+	logger.Info("starting goose migrations")
+	if err := goose.Up(db, "."); err != nil {
+		return fmt.Errorf("goose up failed: %w", err)
 	}
 
-	sort.Strings(files)
-	logger.Info("running SQL migrations", zap.Int("count", len(files)))
-
-	for _, file := range files {
-		logger.Info("executing migration", zap.String("file", filepath.Base(file)))
-
-		content, err := os.ReadFile(file)
-		if err != nil {
-			logger.Fatal("failed to read migration file", zap.String("file", file), zap.Error(err))
-		}
-
-		if _, err := sqlDB.Exec(string(content)); err != nil {
-			logger.Fatal("migration failed",
-				zap.String("file", filepath.Base(file)),
-				zap.Error(err),
-			)
-		}
-
-		logger.Info("migration completed", zap.String("file", filepath.Base(file)))
-	}
+	return nil
 }
 
 // ensureDatabase connects to the default "postgres" database and creates the
 // target database if it does not already exist.
-func ensureDatabase(cfg *config.Config, logger *zap.Logger) {
-	adminDSN := cfg.DSNForDB("postgres")
+func ensureDatabase(cfg dbConfig, logger *zap.Logger) {
+	adminDSN := cfg.dsn("postgres")
 
 	db, err := sql.Open("pgx", adminDSN)
 	if err != nil {
@@ -88,18 +116,31 @@ func ensureDatabase(cfg *config.Config, logger *zap.Logger) {
 	defer func() { _ = db.Close() }()
 
 	var exists bool
-	err = db.QueryRow("SELECT EXISTS(SELECT 1 FROM pg_database WHERE datname = $1)", cfg.DBName).Scan(&exists)
+	err = db.QueryRow("SELECT EXISTS(SELECT 1 FROM pg_database WHERE datname = $1)", cfg.Name).Scan(&exists)
 	if err != nil {
 		logger.Fatal("failed to check database existence", zap.Error(err))
 	}
 
 	if !exists {
-		// CREATE DATABASE cannot run inside a transaction
-		if _, err := db.Exec(fmt.Sprintf(`CREATE DATABASE "%s"`, cfg.DBName)); err != nil {
-			logger.Fatal("failed to create database", zap.String("dbname", cfg.DBName), zap.Error(err))
+		// CREATE DATABASE cannot run inside a transaction.
+		if _, err := db.Exec(fmt.Sprintf(`CREATE DATABASE "%s"`, cfg.Name)); err != nil {
+			logger.Fatal("failed to create database", zap.String("dbname", cfg.Name), zap.Error(err))
 		}
-		logger.Info("database created", zap.String("dbname", cfg.DBName))
+		logger.Info("database created", zap.String("dbname", cfg.Name))
 	} else {
-		logger.Info("database already exists", zap.String("dbname", cfg.DBName))
+		logger.Info("database already exists", zap.String("dbname", cfg.Name))
 	}
 }
+
+// gooseLogAdapter bridges Goose's logger interface to zap.
+type gooseLogAdapter struct {
+	l *zap.Logger
+}
+
+func (a *gooseLogAdapter) Printf(format string, v ...interface{}) {
+	a.l.Info(fmt.Sprintf(format, v...))
+}
+
+func (a *gooseLogAdapter) Fatalf(format string, v ...interface{}) {
+	a.l.Fatal(fmt.Sprintf(format, v...))
+}