fix(contracts): make pricing type an explicit discriminator#357
Merged
Conversation
A contract's pricing was inferred from which of two independent nullable columns (rate, fixed_price) happened to be set, so nothing prevented both from being populated. The document-import path built Contract rows directly (bypassing ContractsIntent validation) and let an LLM-extracted contract carry both a rate and a fixed price into the database. Make `type` (time_based | fixed_price) the single source of truth: - Add ContractType enum + non-null Contract.type column; is_fixed_price now derives from type. validate_pricing() requires the matching value column and clears the other, so an ambiguous row cannot be persisted. - Migration backfills type from existing data (fixed_price wins on any legacy both-set row) and nulls the off-column to clean up contradictions. - Importer derives type when absent and validates on create/update, with enum-string coercion. ContractsIntent delegates to validate_pricing(). - LLM extraction emits type; both contract editors (manual + document import) use an exclusive Time-Based/Fixed-Price toggle. Adds regression coverage for the reported both-set case. Co-authored-by: Cursor <cursoragent@cursor.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
An AI document import produced a contradictory contract: it was both fixed-price (
Fixed Price 2000 EUR) and time-based (Rate 1000 EUR per day). A contract must be exactly one of the two.Root cause was structural: the pricing type was inferred from which of two independent nullable columns (
rate,fixed_price) happened to be set, so nothing prevented both. The document-import path (commit_import→_save_entity) buildsContractrows directly, bypassingContractsIntentvalidation, and the import review UI showed aratefield while the LLM-extractedfixed_pricerode along invisibly.This makes
type(time_based|fixed_price) the single source of truth — exactly the model the frontend already used internally.ContractTypeenum + non-nullContract.type;is_fixed_pricederives fromtype.validate_pricing()requires the matching value column and clears the other, so an ambiguous row can't be persisted.8de1ce679065): backfillstypefrom existing data (fixed price wins on any legacy both-set row) and nulls the off-column to clean up existing contradictions.typewhen absent and validates on create/update (with enum-string coercion);ContractsIntent._validated_savedelegates tovalidate_pricing().type; both contract editors (manual + document import) use an exclusive Time-Based/Fixed-Price toggle that shows Rate or Fixed Price and clears the other.Note for reviewers
For legacy rows (and LLM output) that had both values set, fixed price wins during backfill — the rate is cleared. A contract that should actually be time-based can be switched via the new toggle in the editor.
Test plan
uv run pytest— 340 passedjust check-migrations— applies clean, no drifttsc --noEmit— cleanTestContractPricingInvariant: both-set import normalises to one type, type derived when omitted, model self-heals / rejects missing valueMade with Cursor