Discount governance: ceiling validation + approval routing; one definition of "large deal"

Batch-2 (business completeness) item from the 2026-08-02 review.

Problems

The only discount constraint anywhere is discount <= 100 (quote.object.ts:237). A rep can give 90% off a $99K deal — under the $100K approval threshold, so nothing reviews it. Approval keys on deal amount only (opportunity-approval.flow.ts:461,501).
"What counts as a large deal" is defined three independent times: approval thresholds ($100K/$500K in opportunity-approval.flow.ts), the won-alert threshold (opportunity-won-alert.flow.ts:44), and the sharing rule criteria (sharing/opportunity.sharing.ts:550,568). They will drift.

Scope

Discount ceiling: validation on quote / line-item discount — warn above a soft ceiling, block above a hard ceiling unless an approval reference is present (pick sensible defaults, e.g. 30%/60%, as a single config constant).
Approval on discount: extend the opportunity/quote approval flow to route when max line discount or quote-level discount exceeds the soft ceiling — v17 expression approvers + typed decisionOutputs are available on the pinned protocol and fit this exactly.
Consolidate the three "large deal" thresholds into one shared constants module consumed by the approval flow, the won-alert flow, and (as far as sharing-rule criteria allow referencing it) documented as the single source; pin with a parity test in the spirit of test/priority-rank-parity.test.ts.

Acceptance

A quote with a 90% line discount cannot reach presented without an approval trail.
Changing the large-deal threshold in one place changes every consumer (or the parity test fails).

Ruling (maintainer, 2026-08-11, PM chat, verbatim: 「接受你的全部建议」): implement the minimal version — hard ceiling validation on quote discount + converge the three independently-defined "large deal" thresholds into one config constant (the three-way drift is the structural defect; a validation rule is the declared=enforced shape). The discount-triggered approval routing is deferred — not declined; if a real deal-desk scenario pulls it, it comes back as its own card. Soft/hard ceiling values (default suggestion 30%/60%) are settled in the PR with the reasoning stated.

Queued (M → S-M with the reduced scope). Serial constraint: same schema file as #1017 (quote.object.ts) — #1017 dispatches first, this card follows in a later round.

Generated by Claude Code

Queue hygiene: pm:queue applied, and the serial constraint from the ruling above is now verified cleared.

That ruling recorded "Serial constraint: same schema file as #1017 (quote.object.ts) — #1017 dispatches first, this card follows in a later round." Checked against GitHub rather than carried in memory: #1017 is closed (state_reason: completed), closed by PR #1068 (feat(quote): a quote may not be presented without a recipient), merged. src/objects/quote.object.ts is free, so this card is dispatchable now.

Two notes for whoever takes it:

feat(quote): a quote may not be presented without a recipient (#1017) #1068 just added a requiredWhen-style gate on crm_quote.crm_contact from presented onward. This card also adds validation to the same object. Read feat(quote): a quote may not be presented without a recipient (#1017) #1068's diff before writing — the two rules should read as one coherent set, not as two independently-authored gates that happen to share a file.
⚠️ requiredWhen is a transition gate, not an invariant: a record already in the required state is never re-validated, so every such rule in this repo has a silent legacy hole #1069 is open and directly relevant: it reports that requiredWhen is a transition gate rather than an invariant — a record already in the required state is never re-validated, so every such rule in this repo has a silent legacy hole. The discount-ceiling rule this card adds is in that family. Do not silently inherit the hole; state in the PR which shape you used and why, and if requiredWhen is the wrong instrument for a hard ceiling, say so rather than matching the existing pattern for consistency's sake.

Scope reminder from the ruling: minimal version only — hard ceiling validation on quote discount + converge the three independently-defined "large deal" thresholds into one config constant. The discount-triggered approval routing is deferred, not declined.

Generated by Claude Code

Claim: PM loop round 6

Session: session_01NM6o28jmBgsyTRQHutn7LC
Branch: claude/issue-599-discount-ceiling
Worktree: hotcrm-issue-599
File surface: src/objects/quote.object.ts, the config module that will hold the single "large deal" constant, test/, and — if the rule needs documenting — only the content/docs/sales/quotes.mdx family (.mdx + .zh-Hans + .zh-Hant). Stop on breach; explain in the report.
Container & model: S–M with real design judgment (which validation instrument, and the #1069 hole) — model: opus, mode:subagent.
Serial constraints cleared:

src/objects/quote.object.ts — was held behind crm_quote.crm_contact 要不要自 presented 起 requiredWhen——缺 contact 的报价永远起草不出合同,失败只出现在服务器日志里(源自 #714) #1017; verified: crm_quote.crm_contact 要不要自 presented 起 requiredWhen——缺 contact 的报价永远起草不出合同,失败只出现在服务器日志里(源自 #714) #1017 closed, PR feat(quote): a quote may not be presented without a recipient (#1017) #1068 merged.
⛔ Docs boundary against Billing hand-off: declare outbound webhooks on closed-won and contract activation — CRM scope ends at the contract #600, in flight right now: Billing hand-off: declare outbound webhooks on closed-won and contract activation — CRM scope ends at the contract #600 owns the new billing-hand-off page under content/docs/. This card must not create or edit that page. Different files, so no same-file serialization is required — but the boundary is named here rather than left to chance.
Disjoint from the other in-flight claims: forecast_derive_period：手填 period_start 时 period_end 按「起点 + 3 个月」推导，可越出它自己标注的季度 #1008 (src/objects/forecast.*) and fix(i18n): fill 25 missing ja-JP page-component translation keys #1084/chore(lint): drop --skip-i18n from the lint script — i18n debt is zero, coverage should enter the gate (successor to #494) #1060 (src/translations/ja-JP.ts).

Generated by Claude Code

os-dev-report

{
  "issue": 599,
  "status": "done",
  "branch": "claude/issue-599-discount-ceiling",
  "pr": "https://github.com/objectstack-ai/hotcrm/pull/1088",
  "premise_still_valid": true,
  "summary": "Both halves of the ruling, minimal version. (1) crm_quote gains `discount_within_ceiling`, an error-severity type:'script' validation refusing any quote whose Discount % exceeds 60; it REPLACES `valid_discount` (cut at >100), which was measurably unreachable dead metadata because field bounds are evaluated before object validations, so the field's own max:100 already refused every input that could reach it with code:'max_value'. (2) The large-deal threshold is converged into src/objects/_thresholds.ts. Approval routing on discount is NOT implemented, per the ruling's deferral. PREMISE CHECK: holds, and is larger than the card said — the entry threshold is stated at FOUR sites (approval start condition shared by its _on_create twin, won-alert start condition, and BOTH sharing rules), plus the director tier stated twice more in complementary polarity = 6 executable literals across 3 files, all at the values the card named (100000 / 500000). They had already drifted, in OPERATOR not value: approval and won-alert cut at `>`, both sharing rules at `>=`, so a deal at exactly $100,000 is shared with director+executive but is neither routed for approval nor announced on win. #1069 INSTRUMENT CHOICE (the card's primary design constraint): measured, not matched. A type:'script' validation is a TRUE INVARIANT — evaluated against the merged record on every write, so a row stored above the ceiling before the rule existed is refused on ANY edit until repaired (repair always admitted). A field-level Field.percent({max}) ADMITS every later edit to such a row, i.e. it carries #1069's hole in a second instrument. So the ceiling is a validation rule and max:100 stays only as the arithmetic domain of a percentage; that measurement was added to #1069 as evidence since it generalises its finding past requiredWhen. VALUE CHOICE: 60% hard, and NO soft constant — the soft tier's only consumer was the deferred approval routing, and severity:'warning' was measured inert on this surface (admitted, no warning on the result, no envelope), so shipping 30% would have been declared-but-unenforced vocabulary. 60 rather than 30 because the ceiling is absolute (no approval unblocks it), so it must sit where refusing is unambiguously right. ENVELOPE: followed the landing site's measured shape — name 'ValidationError', code 'VALIDATION_FAILED', fields[].code 'rule_violation'; NO status is asserted anywhere because the error's own keys are exactly ['code','name','fields'] (#1075 is the open card, out of scope). SEEDS: enumerated, deepest discount anywhere in src/data is 20%, so nothing shipped is within 40 points of the ceiling and no migration is needed. DOCS: content/docs/sales/quotes.mdx (+ zh-Hans/zh-Hant) carried a 'High-value discount approval' section describing discount-triggered approval routing, a banner, and a block on Presented — none of which exists; that is the deferred half published as shipped, and it is rewritten to state the real rule. SCOPE NOTE: converging the thresholds necessarily edits the three call-site files (2 flows + sharing), which the dispatch's file-surface list did not name but the ruling requires; flagged in the PR rather than assumed. All named prohibitions honoured (forecast.*, ja-JP.ts and all translation bundles, #600's billing docs, content/docs/releases/).",
  "tests": "All gates run locally in the worktree against main at f71b5350, @objectstack 17.0.0-rc.6, serialized under the shared verification lock. `pnpm validate` exit 0. `pnpm typecheck` (tsc --noEmit) clean — run separately from build, which does not type-check app source. `pnpm lint` exit 0, '178 warning(s), 10 suggestion(s)', all pre-existing (form sections without `name`, empty-position approval nodes); zero hits for discount/threshold in the log. `pnpm hygiene` clean incl. control-byte scan; a separate grep -naP over every touched file found no control bytes. `pnpm build` exit 0 ('17 Objects 345 Fields', '24 Flows'). `pnpm test --maxWorkers=2` exit 0: 'Test Files 88 passed (88) / Tests 2127 passed | 1 skipped (2128)'. New: test/quote-discount-ceiling.test.ts (20 cases) and test/deal-threshold-parity.test.ts (19 cases); test/docs-drift.test.ts rewritten to extract the three amount rules from the COMPILED flow conditions rather than file text (a source regex finds no digits once the value is interpolated). Refusals driven through real ObjectQL on both InMemoryDriver (sparse rows) and real SQLite (column-complete NULLs); every refusal asserted by substance (code + fields[].code + exact message + the record did not move) — no bare toThrow() anywhere, deliberately, because the pre-change schema ALREADY throws on discount:150 via max:100, so a throw-only assertion would have been green against the very defect. REVERSE VERIFICATION, run twice, direction predicted first, both plain red as predicted. (a) Delete the ceiling rule: 'Tests 9 failed | 11 passed (20)' — 'no validation named discount_within_ceiling on crm_quote', \"expected '' to contain 'record.discount > 60'\", and six pins flipping to 'the write was ADMITTED — no refusal to inspect' including the legacy-row #1069 case; the 11 green are the positive paths, the max_value division-of-labour case, the field-max comparison and the seed enumeration. Stated honestly in the PR: with the rule gone the severity:'warning' case passes vacuously and is not load-bearing alone. (b) Re-type one literal (won-alert back to >90000): 'Tests 3 failed | 92 passed (95)' across three independent detectors — parity value ('won-deal alert cuts at 90000, but _thresholds.ts says 100000'), the literal sweep, and docs-drift ('crm_sales.md should state the won-deal alert threshold (one of [\"$90,000\",\"90,000\"])'), the last confirming the rewritten extractor is live rather than vacuous. Fix removed/restored with git checkout + git apply on a saved patch, never git stash; restored tree diffed against the saved patch to confirm byte-identical before the final green run. CONVERGENCE READING (not an assertion): `grep -rnE '\\.amount\\s*(>=|<=|>|<)\\s*[0-9]' src/flows src/sharing` — origin/main 9 hits (6 executable, 3 in comments); this branch 3 hits (0 executable, 3 in comments). The 3 survivors are the #633 totality notes that explain why the comparison aborts on a null; the parity test strips comments before sweeping and asserts 0 executable literals, so the reading cannot silently regress.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #1086: crm_quote_line_item.discount has no ceiling, so a quote at discount:0 with every line at 90% clears the new rule — the line-item half of #599's acceptance criterion, excluded by this card's file surface; also raises a real design question, since a per-line ceiling still permits a deeper EFFECTIVE discount once the quote-level percentage compounds on top",
    "filed as #1087: a deal at exactly $100,000 is shared with director+executive but is neither routed for approval nor announced on win — the four large-deal sites now agree on the value and still disagree on the operator (> vs >=); options and a recommendation (converge on >=) included, since closing it changes who signs what and re-words two published docs tables",
    "commented on #1069 (no new issue): measured evidence that Field.percent({max}) carries the same legacy hole as requiredWhen, and that object validations[] of type:'script' are a true invariant — generalising that card's finding past requiredWhen and giving its option 1 a two-sided rule to document"
  ]
}

Generated by Claude Code

ACCEPT — PR #1088. Reviewed against the PR, not the report. All 9 check runs completed: success.

First, a correction that is mine, not the dev's. My claim comment declared the file surface as quote.object.ts + a config module + tests + the quotes docs. Converging the large-deal threshold necessarily edits its call sites — two flows and the sharing rules — which that list did not name. The dev flagged the divergence in the PR instead of either breaching silently or crippling the work to fit my list. That is the correct handling, and the defective artifact was my surface declaration.

What shipped

crm_quote.discount_within_ceiling — an error-severity type: 'script' validation refusing any quote above 60%. It replaces valid_discount (cut at > 100), which was measurably unreachable dead metadata: field bounds are evaluated before object validations, so the field's own max: 100 already refused every input that could have reached it, with code: 'max_value'. Removing a rule that could never fire is worth more than the rule was.
The large-deal threshold converged into src/objects/_thresholds.ts, with the call sites rewritten to read it.

Approval routing is not implemented, per the ruling's deferral. Verified in the diff.

Three judgments the dev made against the grain, all of which I accept

The instrument, decided by measurement rather than by matching the neighbours. This was the card's primary design constraint. A type: 'script' validation is evaluated against the merged record on every write, so a row stored above the ceiling before the rule existed is refused on any edit until repaired (repair itself is admitted, and that case is pinned). A Field.percent({ max }) admits every later edit to such a row — i.e. it carries requiredWhen is a transition gate, not an invariant: a record already in the required state is never re-validated, so every such rule in this repo has a silent legacy hole #1069's hole in a second instrument. So the ceiling is a validation rule, and max: 100 survives only as the arithmetic domain of a percentage. That measurement was contributed back to requiredWhen is a transition gate, not an invariant: a record already in the required state is never re-validated, so every such rule in this repo has a silent legacy hole #1069, which is the right place: it generalises that card's finding past requiredWhen.
No soft constant, and 60 rather than 30. severity: 'warning' was measured inert on this surface — admitted, no warning on the result, no envelope — so shipping a 30% soft tier would have been declared-but-unenforced vocabulary, the exact thing this card exists to remove. And since the ceiling is absolute (no approval unblocks it, because routing is deferred), it has to sit where refusing is unambiguously right. Both halves reasoned from measurement, not taste.
The premise was larger than the card said. Not three sites but four, plus the director tier twice — six executable literals across three files. And they had already drifted, in operator rather than value: approval and won-alert cut at >, both sharing rules at >=.

Test quality — refusals driven through real ObjectQL on both InMemoryDriver (sparse rows) and real SQLite (column-complete NULLs), each asserted by code + fields[].code + exact message + the record did not move. No bare toThrow() anywhere, and the stated reason is the sharp one: the pre-change schema already threw on discount: 150 via max: 100, so a throw-only assertion would have been green against the very defect. Reverse verification was run twice with the direction predicted in advance, including re-typing one literal to confirm the rewritten docs-drift extractor is live rather than vacuous.

A docs finding worth naming separately from the code. content/docs/sales/quotes.mdx and both Chinese siblings carried a High-value discount approval section describing discount-triggered routing, a banner and a block on Presented — none of which exists. That is the deferred half of this very card published as though shipped. Rewritten to state the real rule.

Out-of-scope findings verified filed (both real, neither folded in): #1086 — crm_quote_line_item.discount has no ceiling, so a quote at discount: 0 with every line at 90% clears the new rule; #1087 — the operator disagreement above, where a deal at exactly $100,000 is shared with director+executive but is neither routed for approval nor announced on win.

⚠️ Scope reading for anyone citing this card later: the ceiling now holds on the quote-level percentage. The line-item path (#1086) is open, and the effective discount compounds. This card's acceptance is met as ruled; the business-level statement "a quote cannot exceed 60%" is not yet true, and #1086 is what would make it so.

Generated by Claude Code

docs(thresholds): _thresholds.ts describes the converged rule, not th…

Navigation Menu

Repository navigation

Discount governance: ceiling validation + approval routing; one definition of "large deal" #599

Problems

Scope

Acceptance

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Discount governance: ceiling validation + approval routing; one definition of "large deal" #599

Description

Problems

Scope

Acceptance

Activity

huangyiirene commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

huangyiirene commented on Aug 11, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions