refactor(progress): SaveProgressRequest becomes the canonical PUT payload; GET serializes from the row

Batch H1 steps 1-2 of the hardening plan. Wire contracts frozen; the pinned shapes are unchanged modulo the whitelisted additive GET keys (docs/contract_pins.md). 1. One canonical request struct. The web PUT binds SaveProgressRequest directly (identity/attribution fields are json:"-" so they stay server-owned) and copies its server fields once — the hand-rolled DTO and the 20-line field-copy block are gone. Parse set for existing clients is IDENTICAL to the old DTO (verified in web/src: the ebook reader sends percentage/current_page/total_pages/reading_mode/epubcfi/ context_text/chapter/zoom_level — reader.ts:3113-3143; the audiobook reader sends percentage/chapter/audio_offset_seconds/elapsed_seconds/ reading_mode — audio.ts:518-520). 2. GET /api/media-items/:id/progress now serializes through one row-struct-driven builder instead of a hand-map, so a row column cannot be silently forgotten again (the audio_offset_seconds omission). Existing keys are verbatim; the row already carried audio_offset_seconds/reading_mode/viewport_x-y/zoom_level/ scroll_position_x-y/panel_number and they surface as WHITELISTED additive keys. The web audiobook reader already reads audio_offset_seconds (audio.ts:219) — with the key absent, durationless books resumed at chapter start (offset always 0); they now resume correctly. The conditional context_text bolt-on and the server-derived css_selector/anchor_href block are unchanged — restore precedence stays percentage-first, never re-derived.
2026-10-16 17:22:32 -04:00
1 parent a2ea34232c
commit 28bd323505
@@ -47,3 +47,14 @@ cover_image_path, has_conflict, format_group, badge`.

 - **H0 (initial pin)** — the shapes above, captured on the unmodified tree
  at commit `b4ae423`.
+- **H1 (progress canonicalization)** — `GET /api/media-items/:id/progress`
+  Whitelisted ADDITIVE keys (fields the `GetUniversalProgressRow` already
+  carries, now surfaced by the shared row serializer):
+  `audio_offset_seconds, reading_mode, viewport_x, viewport_y, zoom_level,
+  scroll_position_x, scroll_position_y, panel_number`. No key removed or
+  renamed; the no-rows and PUT-response shapes are untouched. The web PUT
+  binds `SaveProgressRequest` directly — its parse set for existing
+  clients is IDENTICAL to the old DTO (verified against web/src
+  reader.ts:3113-3143 and audio.ts:518-520); keys the web never sent
+  (viewport_x/y, panel_number) merely become honor-able. All other pins
+  unchanged.
@@ -1154,25 +1154,7 @@ func (mh *MediaHandler) GetMediaReadingProgress(c *echo.Context) error {
 		return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
 	}

-	resp := map[string]interface{}{
-		"id":                  progress.ID,
-		"media_item_id":       progress.MediaItemID,
-		"user_id":             progress.UserID,
-		"current_page":        progress.CurrentPage,
-		"total_pages":         progress.TotalPages,
-		"last_read_at":        progress.LastReadAt,
-		"percentage":          progress.Percentage,
-		"character_offset":    progress.CharacterOffset,
-		"epubcfi":             progress.Epubcfi,
-		"chapter":             progress.Chapter,
-		"chapter_progress":    progress.ChapterProgress,
-		"format_group":        progress.FormatGroup,
-		"total_characters":    progress.TotalCharacters,
-		"chapter_count":       progress.ChapterCount,
-		"last_sync_device":    progress.LastSyncDevice,
-		"last_sync_source":    progress.LastSyncSource,
-		"last_sync_timestamp": progress.LastSyncTimestamp,
-	}
+	resp := progressResponse(progress)

 	// Position authority: re-verify the stored anchor and derive the
 	// client-facing handles (cssSelector + anchor document href) server-
@@ -1208,6 +1190,44 @@ func (mh *MediaHandler) GetMediaReadingProgress(c *echo.Context) error {
 	return c.JSON(http.StatusOK, resp)
 }

+// progressResponse serializes a progress row to the client wire shape —
+// one place, keyed off the row struct, so new row columns surface here by
+// construction. Every existing key is preserved verbatim; the row-carried
+// locator fields (audio/reading_mode/viewport/zoom/scroll/panel) are
+// additive keys per docs/contract_pins.md (H1).
+func progressResponse(progress database.GetUniversalProgressRow) map[string]interface{} {
+	return map[string]interface{}{
+		"id":               progress.ID,
+		"media_item_id":    progress.MediaItemID,
+		"user_id":          progress.UserID,
+		"current_page":     progress.CurrentPage,
+		"total_pages":      progress.TotalPages,
+		"last_read_at":     progress.LastReadAt,
+		"percentage":       progress.Percentage,
+		"character_offset": progress.CharacterOffset,
+		"epubcfi":          progress.Epubcfi,
+		"chapter":          progress.Chapter,
+		"chapter_progress": progress.ChapterProgress,
+		"format_group":     progress.FormatGroup,
+		"total_characters": progress.TotalCharacters,
+		"chapter_count":    progress.ChapterCount,
+		"last_sync_device": progress.LastSyncDevice,
+		"last_sync_source": progress.LastSyncSource,
+		"last_sync_timestamp": progress.LastSyncTimestamp,
+		// Row-carried fields surfaced additively (H1) — the web audiobook
+		// reader already reads audio_offset_seconds (audio.ts) and the
+		// app parses the same names.
+		"audio_offset_seconds": progress.AudioOffsetSeconds,
+		"reading_mode":         progress.ReadingMode,
+		"viewport_x":           progress.ViewportX,
+		"viewport_y":           progress.ViewportY,
+		"zoom_level":           progress.ZoomLevel,
+		"scroll_position_x":    progress.ScrollPositionX,
+		"scroll_position_y":    progress.ScrollPositionY,
+		"panel_number":         progress.PanelNumber,
+	}
+}
+
 // UpdateMediaReadingProgress handles PUT /api/media-items/:id/progress
 func (mh *MediaHandler) UpdateMediaReadingProgress(c *echo.Context) error {
 	userID := c.Get("user_id").(string)
@@ -1246,62 +1266,22 @@ func (mh *MediaHandler) UpdateMediaReadingProgress(c *echo.Context) error {
 		}
 	}

-	var req struct {
-		CurrentPage     *int32   `json:"current_page"`
-		TotalPages      *int32   `json:"total_pages"`
-		Epubcfi         *string  `json:"epubcfi"`
-		ContextText     *string  `json:"context_text"`
-		Percentage      *float64 `json:"percentage"`
-		Chapter         *int     `json:"chapter"`
-		ChapterProgress *float64 `json:"chapter_progress"`
-		CharacterOffset *int64   `json:"character_offset"`
-		ReadingMode     *string  `json:"reading_mode"`
-		ZoomLevel       *float64 `json:"zoom_level"`
-		ScrollX         *float64 `json:"scroll_position_x"`
-		ScrollY         *float64 `json:"scroll_position_y"`
-		// Audiobook locator: seconds into the current chapter
-		AudioOffsetSeconds *int `json:"audio_offset_seconds"`
-		// Honest wall-clock playing seconds since the last push (audio
-		// pushes fire while playing; pauses don't count)
-		ElapsedSeconds *int32 `json:"elapsed_seconds"`
-	}
+	// The canonical write struct binds the wire directly; identity and
+	// attribution fields below are server-owned (json:"-" on the struct)
+	// and copied here once.
+	var req wsync.SaveProgressRequest
 	if err := c.Bind(&req); err != nil {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid request"})
 	}
+	req.MediaItemID = pgtype.UUID{Bytes: mediaUUID, Valid: true}
+	req.UserID = pgtype.UUID{Bytes: userUUID, Valid: true}
+	req.Source = deviceSource
+	req.DeviceID = deviceID
+	req.DeviceType = deviceType
+	req.DeviceName = deviceLabel
+	req.Broadcast = true

 	if mh.progressSvc != nil {
-		saveReq := wsync.SaveProgressRequest{
-			MediaItemID:         pgtype.UUID{Bytes: mediaUUID, Valid: true},
-			UserID:              pgtype.UUID{Bytes: userUUID, Valid: true},
-			Source:              deviceSource,
-			DeviceID:            deviceID,
-			DeviceType:          deviceType,
-			DeviceName:          deviceLabel,
-			Percentage:          req.Percentage,
-			Epubcfi:             req.Epubcfi,
-			ContextText:         req.ContextText,
-			CharacterOffset:     req.CharacterOffset,
-			Chapter:             req.Chapter,
-			ChapterProgress:     req.ChapterProgress,
-			AudioOffsetSeconds:  req.AudioOffsetSeconds,
-			AudioElapsedSeconds: req.ElapsedSeconds,
-			CurrentPage:         nil,
-			TotalPages:          nil,
-			ZoomLevel:           req.ZoomLevel,
-			ScrollX:             req.ScrollX,
-			ScrollY:             req.ScrollY,
-			ReadingMode:         req.ReadingMode,
-			Broadcast:           true,
-		}
-		if req.CurrentPage != nil {
-			cp := int(*req.CurrentPage)
-			saveReq.CurrentPage = &cp
-		}
-		if req.TotalPages != nil {
-			tp := int(*req.TotalPages)
-			saveReq.TotalPages = &tp
-		}
-
 		if req.Percentage != nil && *req.Percentage < 0.005 {
 			existing, err := mh.db.GetUniversalProgress(c.Request().Context(), database.GetUniversalProgressParams{
 				MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
@@ -1312,7 +1292,7 @@ func (mh *MediaHandler) UpdateMediaReadingProgress(c *echo.Context) error {
 			}
 		}

-		result, err := mh.progressSvc.SaveProgress(c.Request().Context(), saveReq)
+		result, err := mh.progressSvc.SaveProgress(c.Request().Context(), req)
 		if err != nil {
 			return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
 		}
@@ -1329,11 +1309,11 @@ func (mh *MediaHandler) UpdateMediaReadingProgress(c *echo.Context) error {
 	}
 	currentPage := int32(0)
 	if req.CurrentPage != nil {
-		currentPage = *req.CurrentPage
+		currentPage = int32(*req.CurrentPage)
 	}
 	totalPages := int32(0)
 	if req.TotalPages != nil {
-		totalPages = *req.TotalPages
+		totalPages = int32(*req.TotalPages)
 	}

 	progress, err := mh.db.UpdateUniversalProgress(c.Request().Context(), database.UpdateUniversalProgressParams{
@@ -294,39 +294,43 @@ func NewProgressService(db *database.Queries, connManager *ConnectionManager) *P
 	return &ProgressService{db: db, connManager: connManager}
 }

+// SaveProgressRequest is the CANONICAL progress write payload — the web
+// PUT binds it directly (identity fields are json:"-": server-owned, never
+// client-settable) and every internal writer fills it programmatically.
+// Wire keys are frozen: only additive json names are allowed here.
 type SaveProgressRequest struct {
-	MediaItemID pgtype.UUID
-	UserID      pgtype.UUID
-	Source      string
-	DeviceID    pgtype.UUID
+	MediaItemID pgtype.UUID `json:"-"`
+	UserID      pgtype.UUID `json:"-"`
+	Source      string      `json:"-"`
+	DeviceID    pgtype.UUID `json:"-"`

-	Percentage      *float64
-	Epubcfi         *string
-	ContextText     *string
-	CharacterOffset *int64
-	Chapter         *int
-	ChapterProgress *float64
-	CurrentPage     *int
-	TotalPages      *int
-	ViewportX       *float64
-	ViewportY       *float64
-	ZoomLevel       *float64
-	ScrollX         *float64
-	ScrollY         *float64
-	PanelNumber     *int
-	ReadingMode     *string
+	Percentage      *float64 `json:"percentage"`
+	Epubcfi         *string  `json:"epubcfi"`
+	ContextText     *string  `json:"context_text"`
+	CharacterOffset *int64   `json:"character_offset"`
+	Chapter         *int     `json:"chapter"`
+	ChapterProgress *float64 `json:"chapter_progress"`
+	CurrentPage     *int     `json:"current_page"`
+	TotalPages      *int     `json:"total_pages"`
+	ViewportX       *float64 `json:"viewport_x"`
+	ViewportY       *float64 `json:"viewport_y"`
+	ZoomLevel       *float64 `json:"zoom_level"`
+	ScrollX         *float64 `json:"scroll_position_x"`
+	ScrollY         *float64 `json:"scroll_position_y"`
+	PanelNumber     *int     `json:"panel_number"`
+	ReadingMode     *string  `json:"reading_mode"`
 	// Audiobook locator (seconds into the current chapter)
-	AudioOffsetSeconds *int
+	AudioOffsetSeconds *int `json:"audio_offset_seconds"`

 	// AudioElapsedSeconds: the player's honest wall-clock PLAYING time
 	// since the last push (audio pushes fire while playing; pauses don't
 	// count). The capture prefers it over the inter-push delta, capped to
 	// the same 2h attributed ceiling.
-	AudioElapsedSeconds *int32
+	AudioElapsedSeconds *int32 `json:"elapsed_seconds"`

-	DeviceType string
-	DeviceName string
-	Broadcast  bool
+	DeviceType string `json:"-"`
+	DeviceName string `json:"-"`
+	Broadcast  bool   `json:"-"`

 	// ClearLocator declares that this write INTENDS to carry no position
 	// locator: percentage-only writers (devices whose converter declined,
@@ -336,7 +340,7 @@ type SaveProgressRequest struct {
 	// client-side far-anchor restore floor. Locater-less rows are honest:
 	// percentage restores are approximate, and the next anchor-bearing
 	// write re-establishes the pair. Request-supplied fields always win.
-	ClearLocator bool
+	ClearLocator bool `json:"-"`
 }

 func (s *ProgressService) SaveProgress(ctx context.Context, req SaveProgressRequest) (database.ReadingProgress, error) {