You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add an offline parity test for the bazaar_intents, bazaar_quotes, and agent_history column definitions duplicated between the initial migration and initBazaarTables().
Compare complete column declarations, including types and defaults.
Align runtime min_rate with the migration: DECIMAL(10,6).
Approach
I chose the issue’s lower-risk “keep both but prove they agree” option. Bazaar initialization currently begins during route registration, while runMigrations() is called afterward. Making the migrator the sole owner would also require changing startup sequencing, expanding this fix’s scope.
The new test reads both definitions directly and runs offline, so future column drift fails CI without requiring PostgreSQL.
Verification
Baseline at 493b11af88aacc86abdc354f10ba8d30761556af:
The new test failed for bazaar_intents.
It identified runtime DECIMAL(5,4) versus migration DECIMAL(10,6).
The bazaar_quotes and agent_history comparisons passed.
Mutation check:
Restoring DECIMAL(5,4) caused only the bazaar_intents parity test to fail.
Restoring the fix returned all three comparisons to green.
Final checks:
Strict TypeScript checks passed for packages/types, packages/sdk, and apps/api.
API suite: 26 test files passed; 185 tests passed; one existing skip; zero failures.
Two things I want to name, because they are why this was picked over the other PR on the same issue.
You took the issue's second option and argued it instead of assuming it. The reason you gave is the real one: ensureBazaarInitialized() runs during route registration, while runMigrations() runs afterwards in index.ts, so making the migrator the sole owner means changing startup sequencing too. The other PR on #31 took the first option without touching that ordering, which would have thrown on a clean database and latched the bazaar off until the next deploy.
And the parity test found a real divergence rather than just guarding a hypothetical one: min_rate was DECIMAL(5,4) at runtime against DECIMAL(10,6) in the migration — a rate of 1.5 would not have fit. The mutation check you describe (restore the old type, watch only that one comparison fail) is the part that tells me the test actually bites.
The test reads both definitions from disk and runs offline, so drift now breaks CI without a Postgres. That was the point of the issue. CI green on all three gates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bazaar_intents,bazaar_quotes, andagent_historycolumn definitions duplicated between the initial migration andinitBazaarTables().min_ratewith the migration:DECIMAL(10,6).Approach
I chose the issue’s lower-risk “keep both but prove they agree” option. Bazaar initialization currently begins during route registration, while
runMigrations()is called afterward. Making the migrator the sole owner would also require changing startup sequencing, expanding this fix’s scope.The new test reads both definitions directly and runs offline, so future column drift fails CI without requiring PostgreSQL.
Verification
Baseline at
493b11af88aacc86abdc354f10ba8d30761556af:bazaar_intents.DECIMAL(5,4)versus migrationDECIMAL(10,6).bazaar_quotesandagent_historycomparisons passed.Mutation check:
DECIMAL(5,4)caused only thebazaar_intentsparity test to fail.Final checks:
packages/types,packages/sdk, andapps/api.Closes #31