refactor: model history table with more fields #189

danielwpz · 2025-11-20T01:54:48Z

Note

Refactors model history to a richer table and repository with audit tracking, updates admin APIs to record/expose changes (including soft delete reason), and adds seed tooling, tests, and Makefile/README updates.

Database:
- Migration: Rename model_pricing_history → model_history; add model_name, model_icon, verifiable, is_active, changed_by_user_id, changed_by_user_email; drop trigger and changed_by.
- Repositories: Record history in application code within transactions (close previous, insert new); support soft delete with default/custom changeReason; new history getters and mappings.
- Tooling: Add seed binary to run migrations and SQL seeds; add mock admin seed (001__seed_mock_admin.sql); add tracing-subscriber dep.
API/Services:
- Extend UpdateModelApiRequest with changeReason; add DeleteModelRequest (optional reason).
- Admin routes now pass changed_by_user_id/email and changeReason on upsert/delete; delete accepts optional body; history response includes modelName, modelIcon, verifiable, isActive, changedByUserId/Email.
- Update service/ports to carry audit fields across layers.
Tests:
- Add E2E tests for model history creation, update sequencing, soft delete (with/without reason), and user audit tracking.
Tooling/Docs:
- New Makefile (dev/test/lint/fmt/preflight) and README updates to use make commands.

^{Written by Cursor Bugbot for commit e782ea5. This will update automatically on new commits. Configure here.}

cursor

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

crates/database/src/repositories/model.rs

crates/database/src/migrations/sql/V0028__refactor_model_pricing_history_to_model_history.sql

crates/database/src/repositories/model.rs

Copilot

Pull Request Overview

This PR refactors the model pricing history table into a comprehensive model history table that captures complete snapshots of model state changes with full audit tracking capabilities.

Key Changes:

Renamed model_pricing_history to model_history and expanded it to include all model fields (name, icon, verifiable, is_active) for complete state snapshots
Replaced generic changed_by string field with structured user audit fields (changed_by_user_id, changed_by_user_email)
Updated model upsert and soft delete operations to automatically record history with change reasons and user context
Added changeReason parameter to admin model update/delete APIs and DeleteModelRequest type for optional delete reasons

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`crates/database/src/migrations/sql/V0023__refactor_model_pricing_history_to_model_history.sql`	Migration to rename table, add new columns (model_name, model_icon, verifiable, is_active), backfill data, rename indexes, drop old trigger/function, and add user audit columns
`crates/database/src/models.rs`	Renamed `ModelPricingHistory` to `ModelHistory` and added new fields (model_name, model_icon, verifiable, is_active, changed_by_user_id, changed_by_user_email) plus audit fields to `UpdateModelPricingRequest`
`crates/database/src/repositories/model.rs`	Added `record_model_history` helper method to close previous history and insert new snapshots; updated `upsert_model_pricing` and `soft_delete_model` to record history; renamed query methods; added default soft delete reason constant
`crates/database/src/repositories/admin_composite.rs`	Updated to pass through new audit fields (change_reason, changed_by_user_id, changed_by_user_email) and map expanded history fields in responses
`crates/services/src/admin/ports.rs`	Added audit fields to `UpdateModelAdminRequest`, expanded `ModelHistoryEntry` with new fields, and updated `soft_delete_model` and `delete_model` signatures to accept audit parameters
`crates/services/src/admin/mod.rs`	Updated `delete_model` implementation to accept and pass through change_reason and user audit parameters
`crates/api/src/routes/admin.rs`	Extract admin user context in `batch_upsert_models` and `delete_model`; pass user ID/email for audit tracking; updated `get_model_history` response mapping; added `DeleteModelRequest` body parameter
`crates/api/src/models.rs`	Added `changeReason` field to `UpdateModelApiRequest`, created `DeleteModelRequest` type, and expanded `ModelHistoryEntry` with new fields matching database schema
`crates/api/tests/model_history_test.rs`	Comprehensive E2E tests covering history creation, updates with proper ordering, soft delete with/without custom reasons, user tracking, and progression through multiple updates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/api/src/routes/admin.rs

crates/api/tests/model_history_test.rs

crates/database/src/repositories/model.rs

Makefile

Copilot

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

crates/database/src/repositories/model.rs

Copilot · 2025-12-01T07:56:56Z

crates/database/src/repositories/model.rs

+                        input_cost_per_token = COALESCE(EXCLUDED.input_cost_per_token, models.input_cost_per_token),
+                        output_cost_per_token = COALESCE(EXCLUDED.output_cost_per_token, models.output_cost_per_token),
+                        model_display_name = COALESCE(EXCLUDED.model_display_name, models.model_display_name),
+                        model_description = COALESCE(EXCLUDED.model_description, models.model_description),
+                        model_icon = COALESCE(EXCLUDED.model_icon, models.model_icon),
+                        context_length = COALESCE(EXCLUDED.context_length, models.context_length),
+                        verifiable = COALESCE(EXCLUDED.verifiable, models.verifiable),
+                        is_active = COALESCE(EXCLUDED.is_active, models.is_active),


Using COALESCE(EXCLUDED.*, models.*) in the ON CONFLICT UPDATE clause means that NULL values in the insert will preserve existing values. However, this conflicts with the partial update pattern used in the UPDATE branch (lines 226-235) where NULL means 'no change'. For consistency and to avoid confusion, the INSERT path should use the same semantics. Consider using separate code paths or explicitly handling which fields can be NULL vs which represent 'no update'.

Suggested change

input_cost_per_token = COALESCE(EXCLUDED.input_cost_per_token, models.input_cost_per_token),

output_cost_per_token = COALESCE(EXCLUDED.output_cost_per_token, models.output_cost_per_token),

model_display_name = COALESCE(EXCLUDED.model_display_name, models.model_display_name),

model_description = COALESCE(EXCLUDED.model_description, models.model_description),

model_icon = COALESCE(EXCLUDED.model_icon, models.model_icon),

context_length = COALESCE(EXCLUDED.context_length, models.context_length),

verifiable = COALESCE(EXCLUDED.verifiable, models.verifiable),

is_active = COALESCE(EXCLUDED.is_active, models.is_active),

input_cost_per_token = CASE WHEN EXCLUDED.input_cost_per_token IS NOT NULL THEN EXCLUDED.input_cost_per_token ELSE models.input_cost_per_token END,

output_cost_per_token = CASE WHEN EXCLUDED.output_cost_per_token IS NOT NULL THEN EXCLUDED.output_cost_per_token ELSE models.output_cost_per_token END,

model_display_name = CASE WHEN EXCLUDED.model_display_name IS NOT NULL THEN EXCLUDED.model_display_name ELSE models.model_display_name END,

model_description = CASE WHEN EXCLUDED.model_description IS NOT NULL THEN EXCLUDED.model_description ELSE models.model_description END,

model_icon = CASE WHEN EXCLUDED.model_icon IS NOT NULL THEN EXCLUDED.model_icon ELSE models.model_icon END,

context_length = CASE WHEN EXCLUDED.context_length IS NOT NULL THEN EXCLUDED.context_length ELSE models.context_length END,

verifiable = CASE WHEN EXCLUDED.verifiable IS NOT NULL THEN EXCLUDED.verifiable ELSE models.verifiable END,

is_active = CASE WHEN EXCLUDED.is_active IS NOT NULL THEN EXCLUDED.is_active ELSE models.is_active END,

fixed via a shared table lock

crates/database/src/repositories/model.rs

crates/api/tests/model_history_test.rs

crates/database/src/migrations/sql/V0028__refactor_model_pricing_history_to_model_history.sql

think-in-universe · 2025-12-01T07:48:45Z

Makefile

+	@echo "Formatting code with rustfmt..."
+	cargo fmt
+
+check-clippy:


check-clippy is identical to lint?

think-in-universe · 2025-12-01T08:21:04Z

crates/database/src/bin/seed.rs

@@ -0,0 +1,100 @@
+use std::fs;


Should we move the seed related code and SQLs to tests folder as it's only needed for testing?

it's also used by local development as well, when you try to create a new model

… migration

PierreLeGuen · 2025-12-01T22:03:38Z

Makefile

@@ -0,0 +1,94 @@
+.PHONY: help seed api dev build test test-unit test-integration lint fmt check-fmt preflight clean


Should we use https://github.com/sagiegurari/cargo-make ?

haven't used that one before, if it provides more benefits than makefile we can definitely switch to it

danielwpz had a problem deploying to Cloud API test env November 20, 2025 01:54 — with GitHub Actions Failure

danielwpz mentioned this pull request Nov 20, 2025

feat: model history with more fields #148

Closed

danielwpz requested review from PierreLeGuen, hanakannzashi and think-in-universe November 20, 2025 01:56

cursor bot reviewed Nov 20, 2025

View reviewed changes

crates/database/src/repositories/model.rs Show resolved Hide resolved

crates/database/src/repositories/model.rs Show resolved Hide resolved

danielwpz had a problem deploying to Cloud API test env November 20, 2025 01:58 — with GitHub Actions Failure

cursor bot reviewed Nov 20, 2025

View reviewed changes

crates/database/src/migrations/sql/V0028__refactor_model_pricing_history_to_model_history.sql Show resolved Hide resolved

crates/database/src/repositories/model.rs Show resolved Hide resolved

think-in-universe requested a review from Copilot November 20, 2025 02:38

Copilot started reviewing on behalf of think-in-universe November 20, 2025 02:38 View session

Copilot finished reviewing on behalf of think-in-universe November 20, 2025 02:41

Copilot AI reviewed Nov 20, 2025

View reviewed changes

crates/api/src/routes/admin.rs Outdated Show resolved Hide resolved

crates/api/tests/model_history_test.rs Outdated Show resolved Hide resolved

crates/api/tests/model_history_test.rs Outdated Show resolved Hide resolved

danielwpz had a problem deploying to Cloud API test env November 20, 2025 02:45 — with GitHub Actions Failure

danielwpz force-pushed the model_history branch from 9b0060c to 704c8c5 Compare November 20, 2025 04:04

danielwpz had a problem deploying to Cloud API test env November 20, 2025 04:04 — with GitHub Actions Failure

cursor bot reviewed Nov 20, 2025

View reviewed changes

crates/database/src/repositories/model.rs Show resolved Hide resolved

danielwpz force-pushed the model_history branch from 704c8c5 to 038ad4d Compare November 20, 2025 05:24

danielwpz temporarily deployed to Cloud API test env November 20, 2025 05:24 — with GitHub Actions Inactive

think-in-universe had a problem deploying to Cloud API test env November 20, 2025 15:07 — with GitHub Actions Failure

think-in-universe had a problem deploying to Cloud API test env November 21, 2025 01:47 — with GitHub Actions Failure

think-in-universe had a problem deploying to Cloud API test env November 21, 2025 02:04 — with GitHub Actions Failure

danielwpz force-pushed the model_history branch from 7a67c8a to c74cccb Compare November 21, 2025 02:24

danielwpz temporarily deployed to Cloud API test env November 21, 2025 02:24 — with GitHub Actions Inactive

danielwpz had a problem deploying to Cloud API test env November 25, 2025 02:53 — with GitHub Actions Failure

danielwpz mentioned this pull request Nov 25, 2025

Issue: Model API endpoints do not record or return the user who made changes #94

Closed

cursor bot reviewed Nov 25, 2025

View reviewed changes

crates/database/src/repositories/model.rs Show resolved Hide resolved

danielwpz force-pushed the model_history branch from 27323f9 to cdc1107 Compare November 25, 2025 03:05

danielwpz temporarily deployed to Cloud API test env November 25, 2025 03:05 — with GitHub Actions Inactive

cursor bot reviewed Nov 25, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

danielwpz temporarily deployed to Cloud API test env November 26, 2025 02:39 — with GitHub Actions Inactive

think-in-universe had a problem deploying to Cloud API test env December 1, 2025 03:19 — with GitHub Actions Failure

think-in-universe requested a review from Copilot December 1, 2025 07:54

Copilot started reviewing on behalf of think-in-universe December 1, 2025 07:54 View session

think-in-universe had a problem deploying to Cloud API test env December 1, 2025 07:54 — with GitHub Actions Failure

Copilot finished reviewing on behalf of think-in-universe December 1, 2025 07:56

Copilot AI reviewed Dec 1, 2025

View reviewed changes

think-in-universe requested changes Dec 1, 2025

View reviewed changes

danielwpz added 12 commits December 1, 2025 18:00

feat: model history table

0e20398

format

cff1966

fix clippy

6c07e12

fix: model history atomicity, temporal gaps, and race conditions

4d9cc80

refactor: rename migration to V0024 to avoid conflict

fee0334

refactor: rename migration to V0025 to avoid conflict with user_agent…

755cb44

… migration

update migration

400bb65

feat: add database seeding binary and Makefile development tools

f13955c

fix: replace misspelled "Superceded" with simpler "Replaced"

762c9ae

Simplify admin model upsert locking

3692373

Renumber migrations for model history refactor

78a1799

fix migration number

0783225

danielwpz force-pushed the model_history branch from c1d7138 to 0783225 Compare December 1, 2025 10:05

danielwpz had a problem deploying to Cloud API test env December 1, 2025 10:05 — with GitHub Actions Failure

format

7dc773d

danielwpz temporarily deployed to Cloud API test env December 1, 2025 10:08 — with GitHub Actions Inactive

update makefile

e62ff31

danielwpz had a problem deploying to Cloud API test env December 1, 2025 10:21 — with GitHub Actions Failure

update makefile

e782ea5

danielwpz temporarily deployed to Cloud API test env December 1, 2025 10:25 — with GitHub Actions Inactive

PierreLeGuen reviewed Dec 1, 2025

View reviewed changes

		@@ -0,0 +1,94 @@
		.PHONY: help seed api dev build test test-unit test-integration lint fmt check-fmt preflight clean

refactor: model history table with more fields #189

Are you sure you want to change the base?

refactor: model history table with more fields #189

Uh oh!

Conversation

danielwpz commented Nov 20, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This PR is being reviewed by Cursor Bugbot

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

danielwpz Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

think-in-universe Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

danielwpz Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

think-in-universe Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

danielwpz Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

PierreLeGuen Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

danielwpz Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danielwpz commented Nov 20, 2025 •

edited by cursor bot

Loading