Repository navigation
The demo makes M3's reminder layer look broken: 54 of 120 contracts have no legal_owner, so four of six scheduled jobs notify nobody #47
Description
Activity
zhuangjianguo commented
on Sep 10, 2026 CollaboratorAuthorMore actionsClaim: PM loop round 4 (seat
repo:hotclm). Dispatched by the PM seat for its dev; the dev inherits this claim and posts no second one.Session:
session_01R3n3GGzobdegM4HUzah1iR
Branch:claude/issue-47-seed-legal-owner
Worktree:hotclm-issue-47(dedicated, offorigin/main@a7b7db5)
Domain:repo:hotclm(single-lane repo — nodomain:*axis)
File surface:src/data/**— the fixture that deals contract ownership, plusREADME.md's operator table only if the dealing introduces a name an operator must create (stop on breach; explain in the report)
Container & model:M,mode:subagent,model: default judgement tier— the card carries a judgement the fix can get wrong (see below), so ⛔ not the floor tier.
Clause-②: no — dealing values across existing seeded rows neither relaxes an acceptance set nor widens a published surface.
Thread-read: none (no comments on this card at claim time, 2026-09-10T02:5xZ)
Serial constraints cleared: #50 (measurement only, edits nothing) and #49 (scripts/demo.mjs) are in the same batch.⚠️ #50 boots the app and measures the seed'sstatuswrites; this card changes the seed'slegal_owner. Different columns, and #50 readsorigin/main, ⛔ not this branch — so neither perturbs the other. One open PR, #40 (DESIGN.md, awaiting a human merge) — no overlap.⚠️ The judgement this card can get wrong, restated because it is the whole reason it is not the floor tier: the obvious fix — give every contract alegal_owner— is wrong. It would stop the "nobody to tell" partitioned edge from ever being exercised, and card 09 measured that edge working. Leave a small, deliberate, commented ownerless set so a future regression there is still visible, and say at the site that it is deliberate. ⛔ Changing the flows is out of scope: this card moves data, and the flows are correct as built.
Generated by Claude Code
zhuangjianguo commented
on Sep 10, 2026 CollaboratorAuthorMore actionsReview: ACCEPT — PR #55, head
a3b7af1, CI 2/2Validatecompleted/success. Merging under theAGENTS.mddelegation.Scope of what I verified myself (⛔ not read off the report): path surface, governed-surface cleanliness, the flows/hook prohibition, the diff's shape, and the assertion guards. ⛔ I did not re-boot to re-count rows — the per-stage and receipt figures are the run's measurements, and I say so rather than implying I re-drove them.
Check Method Result Head binding git rev-parsevs the PR objectboth a3b7af153f9b…; checks bound to that headPath surface git diff --name-onlyREADME.md·src/data/contract.seed.ts·src/data/keys.ts·src/data/plan-contracts.tsGoverned surface grep against the AGENTS.mdset0 hits Zone 1 prohibition grep for ^src/flows/andhook0 — the flows and hooks are untouched, as ruled The guarded ownerless row read the source real throws, ⛔ not comments — see belowIt corrected three of my numbers, and one correction changed the fix
- 「54 of 120 have no
legal_owner」 → 51, corroborated against the compiled artifact. - 「all 12 receipts carry the dev admin」 → 44 receipts, 7 on the dev admin (5 from F3, 2 from F10).
- 「four of six jobs notify nobody」 → one does, F12, for the reason my own card excluded as out of scope.
- 「F3 selected 10, notified 5」 → reproduces exactly.
My arithmetic was measured before #26's
owner_iddeal was in the database. The premise held; the numbers did not, and the run reported that rather than repeating me.⭐ 「51 is not the number to fix」 — the judgement I did not articulate and it did
My Zone 1 said distribution, not saturation. The run found the sharper rule: drafts,
cancelled,submitted-but-unpicked (§05's 待受理 queue) and every contract on a type that skips legal review are ownerless because F2 never assigns one to them. Filling those would be inventing history.⇒ Only the twelve at
in_reviewcost a notification, because F3 is the only job whose recipients arelegal_owneralone. That is a better statement of my own constraint than my own constraint was.And the deal itself follows
ownerOf's existing rule — by counterparty relationship — rather than a fresh scheme, with the comment stating plainly that it does not claim to reproduce F2 (which picks the counsel with the fewest open contracts at the moment of submission, an answer that needs 68 days of history a one-pass corpus does not have). Declining to imply a replay is exactly the annotation discipline this repo keeps getting bitten by.⭐ The finding my card did not have, and it is the entire 5-of-10
Six of the twelve
in_reviewcontracts sat onSOW/ORD— types whoserequiresLegalReviewisfalse.contract_state_machinerefusessubmitted → in_reviewon those. So those six were rows no surface in this app could have produced, and they are exactly the six F3 selects and can tell nobody about.plan.ts's own stated property 1 — no row is in a state the write layer would have refused — had a third uncovered edge.⛔ And it refused the cosmetic fix, in its own words: giving those six an owner "would have swapped one impossible claim for another (a legal owner on a type F2 skips) and let the fixture keep lying while the number looked better." Instead
dealStatusesswaps them onto legal-review types, andassertDealtStatesre-proves both per-row invariants after all three passes — because a swap moves two rows and counting statuses cannot notice a repair undoing an earlier one. That last sentence is the kind of thing that only comes from actually thinking about the order of operations.The deliberate ownerless row is provable, not hopeful
I asked for 「a small, deliberate, commented ownerless set」. What landed is asserted:
handBackInReviewthrows if too few contracts sit in F3's(-60, -30]band with an owner — and the band reasoning is exact: past 30 so the over-30 stage selects it, and inside 60 because the over-60 stage copiesclm_legal_head, which would hand the notification a recipient and take the notify edge instead. Only the over-30 stage haslegal_owneras its sole recipient.assertDealtStatesthrows on the newly-foundin_review-on-a-non-review-type edge, and on the pre-existing blocked-counterparty one.- One is minimum and maximum, because every extra handed-back row is a reminder the demo does not send — the defect this card exists to fix.
⇒ A future reader who "tidies up" that row hits an error message telling them they are deleting the only place this corpus exercises the quiet edge. That is coverage that defends itself.
Two pieces of honest reporting that would have been easy to skip
F3's
selectedfell 10 → 6, and the run called it noise rather than a win.timelineFordraws eachin_reviewcontract's review age from −8…−58 days, so the share past 30 is a draw; the status swap shifts the shared PRNG stream. 10-of-12 was a lucky sample. The numbers that are not noise:in_reviewrows carrying an owner 6/12 → 11/12 · F3 selections it can tell someone about 50% → 83% ·legal_owner-addressed receipts on the dev admin 5 of 5 → 0 of 5 · distinct recipients 5 → 7 ·in_reviewrows in a state the app refuses 6 → 0.⛔ And it declined to tune the distribution to make the reminder count look better, citing
AGENTS.md's prohibition on 「a seeded number that looks computed」. Inflating a count against a figureDESIGN.mddoes not pin is precisely the trap.The trade-down is stated, not buried: out of the box this now produces zero legal notices where before it produced five to the dev admin. It argues that is #28's open question rather than a new one —
owner_idhas behaved this way since #26, and out of the box the only existing account is servednavigation: []so nobody can open an inbox to see them anyway. I accept that reading, and #28 is where it belongs.Browser proof is the right proof: signed in as each lawyer, two different inboxes, five notices between them, each confirmed by a
user_id-filtered receipt query — which is what distinguishes genuinely two recipients from one list rendered twice.Findings it filed, and one it correctly declined to fix
#52 (
clm_obligation.owneron all 200 rows — the last receipts left on the dev admin), #53 (34 backfilled contracts carrying alegal_ownerwhileis_backfilledsays no review happened, with both readings and their costs), #54 (scripts/demo.mjs's two now-stale sentences). All three exist; I am triaging them.⭐ #54 was held rather than fixed because PR #51 owned
scripts/demo.mjsthis round. A dev noticing a cross-PR file collision and declining to edit into it is serial discipline I did not have to enforce.Landing
CI green ✓ · ACCEPT recorded ✓ · no governed surface ✓ (verified) · no REWORK round ✓ ⇒ squash-merge.
⚠️ Note for the queue: this PR touchesREADME.md, which is exactly the collision that kept #46 held this round. #46 goes out once this lands.
Generated by Claude Code
- 「54 of 120 have no
- added a commit that references this issue
on Sep 10, 2026 zhuangjianguo commented
on Sep 10, 2026 CollaboratorAuthorMore actionsos-dev-report
{ "issue": 47, "status": "done", "branch": "claude/issue-47-seed-legal-owner", "pr": "https://github.com/objectstack-ai/hotclm/pull/55", "premise_still_valid": true, "summary": "The defect is real and is fixed; three of the card's four figures were stale, and one of them changed the diagnosis. Re-measured on origin/main @ a7b7db5 (clean DB, pnpm demo, README operator setup, one run of each job, read from the driver's own SQLite file): 51 of 120 contracts have no legal_owner, not 54 (the compiled artifact agrees); 44 notification receipts exist, of which 7 carry the dev admin, not 12 of 12 (that reading predates #26's owner_id deal); one of six jobs notifies nobody (F12, for the reason the card itself excludes), not four. F3's 10-selected/5-notified reproduces exactly. What the sharper reading shows: 51 is not the number to fix, because drafts, cancelled, submitted-and-unpicked-up (DESIGN.md section 05's waiting-intake queue) and every contract on a type that skips legal review are ownerless precisely because F2 never assigns one; filling them would invent history. The rows that cost a notification are the twelve at in_review, because F3 is the only job whose recipients are legal_owner alone. Two causes, both data. (1) legal_owner named DEMO_USER on 69 rows; it now names the two clm_legal_counsel accounts of DESIGN.md section 10, dealt by counterparty relationship the way ownerOf deals requesters, 34/34, with the site stating that it approximates rather than replays F2's least-loaded rule and why a one-pass corpus cannot replay it. (2) Six of the twelve in_review contracts sat on the two types whose requiresLegalReview is false: contract_state_machine refuses that edge twice over and F2 never assigns a legal owner to such a type, so they were rows no surface in this app could have made AND exactly the rows F3 selects and can tell nobody about. Giving them an owner would have swapped one impossible claim for another, so dealStatuses gains a third repair that swaps them onto legal-review types in the same style as the two repairs already there, and assertDealtStates re-proves both per-row invariants after all three passes. One contract in review is deliberately handed back with no legal owner so F3's partitioned quiet edge stays exercised; handBackInReview proves reachability rather than assuming it, requiring the row to sit in F3's (-60, -30] day band, past 30 so the over-30 stage selects it and inside 60 so the over-60 stage, which copies clm_legal_head and would give the notification a recipient, does not take it first. One is the minimum that keeps the edge live and therefore also the maximum, since every extra handed-back row is a reminder the demo does not send. No file under src/flows/, no hook, no DESIGN.md; README.md's operator table gains the two exact account names because the deal introduces people an operator must create. The assignee field was not touched and no second claim was posted.", "tests": "GATES on a3b7af1, clean tree, exit codes captured before any pipe (cmd > file 2>&1; EXIT=$?): VALIDATE=0 LINT=0 TYPECHECK=0 I18N=0. Judgement lines the gates printed themselves: 'Validation passed (1012ms)'; '21 warning(s), 5 suggestion(s)' which is the same count and the same two families (field-no-consumers, approval-approvers-may-resolve-empty) PR #42 records for origin/main, so this PR adds none; 'i18n gate ... COVERAGE: 0 missing keys across 2 locale(s)'. BEFORE/AFTER, identical protocol both sides (rm -rf .objectstack/data; pnpm demo on port 3147; README operator setup = 3 requesters + all 7 positions + dev admin into clm_admin; re-seed so the upsert hands over; one POST /api/v1/automation/{name}/trigger per job; counts read from each run's own node summary, receipts read from SQLite). SELECTED to NOTIFIED per job, before then after: legal_review_sla over-30 10 to 5 (quiet 5) then 6 to 5 (quiet 1); over-60 0 to 0 then 0 to 0; turn_stalled 1 to 1 then 2 to 2; obligation_due 2/0/0 to 2 then 2/0/0 to 2; payment_overdue 0/18 to 18 acted 18 then 0/18 to 18 acted 18; renewal_notice 21 to 0 then 25 to 0 (unchanged and untouched, the card ruled it correct); expiration_sweep 0/0 to 0 then 0/0 to 0. failed=0 on every run both sides. F3's selected fell 10 to 6 and that is NOISE, not signal: timelineFor draws each in_review review age from -8 to -58 days, and the status swap shifts which contracts are in_review and therefore the shared PRNG stream; 10-of-12 was a lucky sample and 6-of-12 is near the mean. It was deliberately not tuned. The non-noise numbers, before then after: in_review rows carrying an owner 6/12 then 11/12; F3 selections it can tell someone about 50% then 83%; legal_owner-addressed receipts on the dev admin 5 of 5 then 0 of 5; distinct recipients 5 then 7; in_review rows in a state the app refuses 6 then 0. WHERE NOTIFICATIONS LAND, receipts by recipient, before: Finance Controller 18, Business Requester 1 12, Dev Admin 7, BR2 4, BR3 3 (44 total). After: Finance Controller 18, BR1 9, BR2 6, BR3 5, Legal Counsel 2 3, Legal Counsel 1 2, Dev Admin 2 (45 total). By topic: clm_legal_review_over_30_days went from Dev Admin x5 to Legal Counsel 1 x2 plus Legal Counsel 2 x3; the two remaining Dev Admin receipts are clm_obligation_due_soon, i.e. clm_obligation.owner, filed as #52. QUIET EDGE AT ROW LEVEL, after: F3's over-30 stage selected six rows; MSA-2026-0005 'Master Services Agreement - Meridian Retail Group', 56 days in review, legal_owner NULL, sys_notification rows for it = 0; the other five (LSE-2026-0004, ICA-2026-0004, SUP-2026-0001, NDA-2026-0003, NDA-2026-0016) each got exactly 1; run green, over30_tell runs 5 skipped 1. ZONE 2 ASSUMPTION 2, re-measured on 17.4.0 rather than assumed: seeded on a clean database with the counsel accounts ABSENT, all 120 rows land legal_owner NULL and every row survives (120 contracts / 60 reviews / 200 obligations / 300 instalments), so Field.user resolves exactly like Field.lookup('sys_user') and an unresolvable name never refuses the row. Out of the box in that state F3 selects 6 and notifies 0, run green, 0 notifications written. BROWSER, Chromium 1194 at /opt/pw-browsers/chromium-1194/chrome-linux/chrome, playwright install NOT run: signed in as each lawyer, both landing on /_console/apps/clm/clm_contract/view/my_contracts with full navigation. Legal Counsel 1 bell badge 2, panel 'Inbox 2 total 2 notifications 0 pending approvals', showing SUP-2026-0001 and NDA-2026-0003. Legal Counsel 2 bell badge 3, panel '3 total 3 notifications', showing LSE-2026-0004, ICA-2026-0004, NDA-2026-0016. Each panel loaded 200 GET /api/v1/data/sys_notification_receipt filtered on a DIFFERENT user_id, which is what makes them two recipients rather than one list rendered twice; before this PR all five of those rows carried the dev admin's id. Screenshots taken. Console: one 404 for a static asset (present on the baseline per PR #42) and 403 PERMISSION_DENIED on GET /api/v1/data/sys_activity, the platform Console's own activity feed which no clm_* set grants, surfacing only because this is the first browser pass driven as a non-admin; neither is application output and neither is caused by this diff. BOOT: the two known platform ERROR lines (objectstack#17175 _objectstack_sequences, #17176 sys_oauth_resource) plus the pre-existing '[Seeder] Inline seed exceeded 8000ms budget' notice, nothing else; both re-seeds completed with zero 'Failed to write' lines. No ablation applies: this card ships no new guard, and the two compile-time assertions it does add (assertDealtStates, handBackInReview) are proven by the artifact they produce, 11 of 12 in_review rows owned and the handed-back row measured at 56 days inside the (-60,-30] band.", "mcp_calls": "5 - three issue_write create calls (#52, #53, #54), one create_pull_request, and this report comment. Every read went through zero-quota channels: REST for the card, its comments, PR #42, PR #51's diff and the dedup listing; git and the local database for everything else.", "open_questions": [], "out_of_scope_findings": [ "filed as #52: clm_obligation.owner is the dev admin on all 200 seeded obligations, so after this PR F10's are the only receipts left on that account (2 of 45); same shape as this card, different column, and it needs its own answer on how much of F10's quiet edge to keep", "filed as #53: 34 backfilled contracts carry a legal_owner while is_backfilled says on the same row that no review happened, against the field's own description 'The lawyer who accepted the review'; this PR deals the column but deliberately does not change WHICH rows carry it, because clearing them would cost F12/F13 a recipient - DESIGN.md section 10 draws the expiring corpus from exactly those rows", "filed as #54: scripts/demo.mjs's DEMO_USER doc block and its priming-failure message still list clm_contract.legal_owner among the references resolving against the dev admin, which this PR made false; that file is held by PR #51 (#49) this round so it was NOT edited here - this is debris this PR creates and the fix must land after both PRs", "noted, not filed: accounts created through POST /api/v1/auth/admin/create-user land with must_change_password = 1 and every API call then answers 403 PASSWORD_EXPIRED until the password is rotated through POST /api/v1/auth/change-password; sys_user.must_change_password is not writable through the data API (ADR-0092). Not filed because an operator following the README uses Setup and Users, not this endpoint, and whether that path sets the same flag was not measured. Carrier: #45's open dogfood report, which is where operator-experience findings on this tree are being collected", "noted, not filed: the Console's own activity feed answers 403 PERMISSION_DENIED for a signed-in non-admin (GET /api/v1/data/sys_activity), because no clm_* permission set grants it. Platform Console surface, not this app's metadata. Carrier: #45's open dogfood report, whose pass 2 drives the five audience groups" ] }
Generated by Claude Code
- added a commit that references this issue
on Sep 10, 2026
Found in the dogfood pass (#45) on
main@a7b7db5. Seed-data quality, ⛔ not a defect in the jobs — card 09's flows do exactly what they were built to do. The problem is that the corpus they run against gives them nobody to tell.Measured
legal_review_sla(F3) selected 10 contracts over 30 days and produced 5 notify runs. The other five took the partitioned "nobody to tell" edge, because only 5 of those 10 carry alegal_owner. Across the corpus: 54 of 120 contracts have nolegal_ownerat all.One run of each job on the seeded corpus:
legal_review_sla(F3)turn_stalled(F4)obligation_due(F10)payment_overdue(F11)renewal_notice(F12)expiration_sweep(F13)And all 12
sys_notification_receiptrows carried the dev admin'suser_id— so what a demo actually shows is the administrator talking to themselves.Why this costs more than it looks
M3's whole point — its acceptance criterion is 「到期、逾期提醒在收件箱可见」 — is the reminder layer. It landed hours ago and works. But the first thing an evaluator does is open the inbox, and what they see is a handful of notices, half the flagged contracts silently skipped, and every recipient the same account.
The honest reading of that screen is "the reminders are unreliable". The truth is "the demo data has no owners". ⛔ Nothing on screen distinguishes those two, and the one the evaluator will believe is the worse one.
This is the showcase rule biting: an exhibit that under-demonstrates a working capability teaches the reader the capability is weak.
Fix
Deal
legal_owneracross the seeded corpus the wayowner_idis already dealt (#26 established the pattern: the fixture names them, the operator's accounts receive them). Every contract that can reach a reminder path should have someone to notify.legal_owner, the "nobody to tell" edge stops being exercised at all and a real regression there would go unseen. Leave a small, deliberate set ownerless and say so at the site, so the partitioned edge stays demonstrated.F12's
renewal_notice0-of-21 is a separate and correct result — those contracts are excluded by the once-per-contract key because the seed already carriesis_expiring = 1on them. ⛔ Do not "fix" that here; it is not the same defect.Related
#45 (the dogfood pass) · #39 / PR #42 (card 09, the flows themselves — working as built) · #26 (the
owner_iddealing pattern to follow) ·DESIGN.md§10