feat(lead): one duplicate banner per verdict, each with its own next step - #1684
Merged
Merged
Conversation
…step `src/pages/lead_detail.page.ts` carried ONE `record:alert` for both duplicate verdicts. #1207 gated it on `duplicate_status == "suspected"`; #1289 widened it to every verdict the field carries. The widening was right, but a `record:alert` carries a single `visible` and a single title/body pair, and `pickLocalized` picks by LANGUAGE, not by row — so one component covering both verdicts had to word itself so it named NEITHER, or it would have mislabelled every lead in the other state. Since #1288 the two verdicts have opposite next steps: `suspected` warns and conversion PROCEEDS, `confirmed` is REFUSED outright by `refuse_confirmed_duplicate`. One sentence cannot state either without being false for the other, so it stated neither — the banner announced that something was wrong without saying what to do, and the rep had to scroll to the Duplicate Status chip to find out which situation they were in. On `confirmed` it was worse: nothing on the record said the Convert button would refuse them, so they learned it by pressing it. So the banner becomes two, one per verdict, in all four locales. `suspected` keeps `warning` and says to compare against the linked record first; `confirmed` ships at `error` — which `record-alert.tsx` maps to `role="alert"` / `aria-live="assertive"` rather than the polite `role="status"` every other level gets — and is the only place a rep is warned about the refusal before pressing Convert. Two sibling `record:alert` nodes really do both render: a region renders as `components.map((node, i) => <SchemaRenderer key={node?.id || fallback} …>)`, read out of the shipped console bundle at the `.objectui-sha` pin, so each is mounted separately and evaluates its own `visible` against the same row. Their ids are their React keys, which is why the two ids differ. Both halves of the guard #1289 ruled for are intact. `has()` stays verbatim — this call site is FAIL-SOFT, so an unevaluable predicate SHOWS the banner, and an unguarded predicate aborts with `No such key` on every clean lead whose driver omits the column. The comparison beside it becomes the EQUALITY, which is strictly narrower than `!= null` and subsumes it: measured on the pinned engine, `null == "suspected"` is a clean `false`, not a fault, and the two spellings agree on every record shape a driver can produce. The same spelling already ships on this field one file over, in the conversion flow's `e21` / `e25` edges. A lead carrying a value neither option declares now raises NO banner, where the widened predicate raised the neutral one. That matches what the flow already does with such a row — `e22` Clean converts it — so the page and the flow now agree about the same lead. Nothing here changes what the app refuses; that was ruled by #1288 and shipped by PR #1555. The pins in `test/lead-duplicate-visibility.test.ts` move deliberately, three of them: the shape (one alert -> two, addressed by id, ids distinct), the predicate table (each banner true on exactly its own verdict, plus a new mutual-exclusion pin mirroring the flow's one-live-edge rule), and the copy rule, which INVERTS from "names neither verdict" to "names its own and never the other". The copy rule still reads both words out of the locale packs, so renaming an option re-aims the assertion instead of retiring it. Claude-Session: https://claude.ai/code/session_018xtjdpZFjgWh4Ad9Wcx68J Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
os-steve
marked this pull request as ready for review
September 6, 2026 10:18
This was referenced Sep 6, 2026
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
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.
Closes #1628
Splits the lead detail page's duplicate banner into one
record:alertper verdict, so each one states the next step its verdict actually has. No change to what the app refuses.Why one banner could not state either next step
#1207 gated a single banner on
duplicate_status == "suspected"; #1289 widened it to every verdict. Widening was right — but arecord:alertcarries onevisibleand one title/body pair, andpickLocalizedpicks by LANGUAGE, not by row. One component covering both verdicts therefore had to word itself so it named neither, or it would have mislabelled every lead in the other state.Since #1288 the verdicts have opposite next steps:
suspectedconfirmedrefuse_confirmed_duplicate)So the neutral banner announced that something was wrong without saying what to do. On
confirmedit was worse: nothing on the record said Convert would refuse them, so they learned it by pressing it.What ships
lead_duplicate_alert_suspectedsuspectedwarninglead_duplicate_alert_confirmedconfirmederrorAll four locales (
en/zh-CN/ja-JP/es-ES), plus the componentlabelkeys insrc/translations/*/app.ts.lint:i18n-gatereports 0i18n/missing-*.erroris not decoration:RecordAlertPropsdocuments, andrecord-alert.tsximplements,role="alert"/aria-live="assertive"forerroragainst the politerole="status"every other level gets.suspectedstayswarningbecause conversion still goes through.Two sibling
record:alertnodes really do both render — verified before building on itThe whole plan rests on this, so it was measured on the pinned 17.3.0 bundle, not assumed.
RegionContentinnode_modules/@objectstack/console/dist/assets/ui-components-*.jsat the.objectui-shapin (00d3f09c):Every component is mapped to its own renderer, so both banners mount and each evaluates its own
visibleagainst the same row. The React key isnode.id— which is why the two ids differ rather than sharing one, and why that is now pinned.The predicate: guard kept verbatim, comparison narrowed
has(x) && x != null.Both halves of the shape #1289 ruled for are intact — the
has()guard verbatim, and a comparison beside it that makes "set" mean set. What changed is that the comparison became the equality, which is strictly narrower than!= nulland subsumes it. Measured on@objectstack/formula17.3.0:has && != nullhas && == "suspected"has && != null && == "suspected"{}(driver-memory / mongodb)falsefalsefalse{duplicate_status: null}(driver-sql clean)falsefalsefalsesuspectedtruetruetrueconfirmedtruefalsefalsemergedtruefalsefalseThe
!= nullterm is a provable no-op beside the equality — identical verdicts on all five shapes — andnull == "suspected"is a cleanfalseon this engine, not a fault. A dead term on a fail-soft surface is worse than no term: it reads as load-bearing to the next person. The repo's own precedent settles the spelling — the conversion flow'se21/e25edges (#1288), on this same field, already readhas(vars.leadRecord.duplicate_status) && … == "suspected".⛔ Neither half is simplified away, and both remain pinned: bare
has()is still TRUE for a present-and-null key (re-measured), and the unguarded tail still faults on a keyless record.The pins that moved, and why — deliberately, one by one
test/lead-duplicate-visibility.test.ts, no new test file, no new gate (AGENTS.md:431).ships a warning-severity record:alert→ships ONE record:alert per verdict, under distinct ids+ a per-verdict severity pin. The old pin asserted a single alert at one severity; there are now two, at two severities. The distinct-id half is new and load-bearing (React keys, above).answers with a VERDICT on every record shape→ per-banner, table-driven: each banner istrueon exactly its own verdict across all five shapes. The old pin's row 4 asserted the widened banner wastrueonconfirmed; that row now belongs to theconfirmedbanner.at most one banner is ever shown— NEW. The record-page twin of the flow's "exactly one live edge" pin. Two banners on one row would stack two contradictory next steps; nothing structural prevents it, so it is measured.the guard is load-bearing→ per-banner. ⭐ The property Aconfirmedduplicate that is not yet disqualified gets no banner and no conversion warning — #1207's predicates aresuspected-only because my ruling said so #1289 built in is preserved: the unguarded text is still built from the shipped predicate itself (source.split('&&').pop()), never typed out. What splitting changed is what that tail says — it used to be!= null, and is now== "this banner's verdict". So the second leg now asks for the banner's own verdict row: the unguarded== "confirmed"tail is correctlyfalseon a suspected lead, and assertingtruethere would pin the wrong claim. It holds for both predicates.describes the flag without asserting WHICH verdict it is→names ITS OWN verdict, and never the other one. This is the inversion. One banner shown on both states could assert neither; a banner shown on exactly one state must name that state — that is the card. It must still never name the other, for exactly the reason Aconfirmedduplicate that is not yet disqualified gets no banner and no conversion warning — #1207's predicates aresuspected-only because my ruling said so #1289 gave. ⭐ Both words are still READ FROM the locale packs, never typed here, so renaming an option re-aims both assertions instead of retiring them.Unchanged:
names the record it repeatsand everylead_conversionblock.One behaviour change, deliberate
A lead carrying a value neither option declares (e.g.
merged) now raises no banner, where the widened predicate raised the neutral one. The conversion flow'se22Clean edge already treats such a row as clean and converts it — so the page and the flow now agree about the same lead instead of contradicting each other.Evidence
Red before green. Pins changed first, predictions written down, then run against the unchanged page: 18 failures predicted, 18 observed, name for name (1 shape + 2 severity + 2 envelope + 2 verdict-table + 5 exclusivity + 2 guard + 2 locale + 2 copy); the other 20 stayed green. After the page change: 38/38.
Ablation (from the committed implementation; mutation proven on disk by anchor counts and blob hash, restored and proven by an empty
git diff HEAD). Widening theconfirmedpredicate back to!= nullproduced exactly the 3 predicted failures:pnpm verifyfully green — all eight stages:validate·typecheck·lint·lint:i18n-gate(0i18n/missing-*) ·hygiene·hygiene:tokens·build·test(161 files, 3414 passed, 1 skipped). Token ratchet clean: interaction layer 37,366 → 37,963 (ceiling 40,000).Out of scope
duplicate_status: suspectedbe blocked, or only warned about? #1288 decided it, PR Refuse conversion of a confirmed-duplicate lead #1555 implemented it; nothing here changes what the app refuses.properties.sectionsuntouched — crm_lead 有三套互不相同的字段分组(fieldGroups 10 / 详情页 5 / 表单 6),详情页的 Details 标签页够不到 notes、重复管理与沟通偏好 #806's territory,pm:blocked.test/lead-duplicate-visibility.test.tsis named by none of epic Epic: bring this repo's test farm back under the 2026-08-31 ruling — platform-first,os lint --strictfirst, then retire the local re-implementations by family #1579's F3 (Retire the local analytics / dataset integrity tests the platform's chart / widget / dataset rules already enforce (epic #1579, step 3, family F3) #1584) / F5 (Retire the local security / sharing / org-axis coverage tests the platform's security-* and sharing-rule-* rules already enforce (epic #1579, step 3, family F5) #1586) / F6 (Retire the local hook / action / flow write-shape tests the platform's *-body-write-* and flow-node-write rules already enforce (epic #1579, step 3, family F6) #1587) tables, which the dispatch had ruled out by subject rather than by reading.Generated by Claude Code