Repository navigation
The obligation reminders are still the administrator talking to themselves: clm_obligation.owner is the dev admin on all 200 seeded rows - #65
Merged
Conversation
… dev admin All 200 seeded obligations named `DEMO_USER`, so F10's reminders were the administrator writing to themselves — the only notification topic still doing so after #26 dealt `owner_id` and #47 dealt `legal_owner`. Re-measured on `main` @ 2d63324 before changing anything: 200 of 200 on the dev admin, and `clm_obligation_due_soon -> Dev Admin x2` was the last dev-admin receipt in the book. `owner` is optional, so it may name accounts the operator has yet to create. It now names them by what the obligation IS: a `compliance` filing goes to the lawyer who knows that counterparty, a `deliverable` or `report` to the requester who launched the contract. Both are the counterparty-relationship rule `ownerOf` and `legalOwnerOf` already use, so the two locales deal identically, and both accounts are already in the README's exact-name table — this deal adds no new account for the operator to create. Deliberately NOT a copy of the contract's `owner_id`: DESIGN.md §03 declines to default this column to the contract owner, and a fixture that put all 200 on the launching requester would assert by construction what §03 refuses to assert by default. One row is left unassigned, and `leaveUnassigned` proves F10 will select it. All three of F10's stages take `{obligation.owner}` as their sole recipient, so an unowned obligation is the only way this corpus exercises the partitioned "nobody to tell" edge that keeps `loop-node.ts`'s bare `await` from ending a sweep on one unreachable recipient. Its due date is constructed at T+7 rather than drawn: measured, the `today` and `arrears` stages select 0 on a freshly seeded database, so the `week` stage is the only one that can carry the edge. It is taken from the due-soon band so §10's "40 due in the next 30 days" does not move. 820 rows, both locales row-for-row identical, no seeded users. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR
`scripts/demo.mjs` conflicted in the two passages #54 / PR #60 (`6e534689`) also edited: the `DEMO_USER` doc block and the `Dev Admin` name-check failure detail. Both edits move the same way — #60 dropped `clm_contract.legal_owner` from the two reference lists, and this branch takes the same lists one reference further to `clm_review.reviewer` alone, which is the only one left that `required: true` forces to resolve at seed time. Resolved to this branch's wording in both, on top of #60. Nothing that merged in touches `src/data/`, `src/flows/` or the objects, so the fixture measurements in the PR body still describe this tree. Re-verified after the merge anyway: all four gates green, both locales compile to 820 rows with `clm_obligation.owner` positionally identical and the deal unchanged (53 / 50 / 40 / 29 / 27 and one deliberately unassigned). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR
…versal "the only user reference in the fixture that still points at the dev admin" was a universal, and one counterexample kills it: `clm_deviation.decided_by` is `Field.user` (deviation.object.ts) and `negotiation.seed.ts` still stamps it with `DEMO_USER` on every decided deviation. Two references name the account; the sentence claimed one. Scoped to what the paragraph actually supports and what the operator has to act on: `clm_review.reviewer` is the reference that still HAS TO name it, because it is the only `required: true` user reference in the model — checked against all six (`submitted_by`, `owner_id`, `legal_owner`, `decided_by`, `clm_obligation.owner`, `reviewer`), and only `reviewer` carries it. The `required: true` mechanism and the reassign instruction are unchanged. `decided_by` is deliberately left unnamed rather than listed beside it: it is an audit stamp with nothing routed to it and nothing for the operator to reassign, and #61 is queued to re-deal it — naming it here would cost that card a second README edit. Omitting a member is incompleteness; the defect being fixed was the universal, not the omission. `scripts/demo.mjs`'s "the ONLY reference this boot has to satisfy" is correct as it stands and is untouched: it is scoped to what the priming boot must resolve, and `decided_by` is optional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR
zhuangjianguo
marked this pull request as ready for review
September 10, 2026 09:08
This was referenced Sep 10, 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.
Fixes #52
Data only. Five files: three in
src/data/, plusREADME.md(the operator table) andscripts/demo.mjs(its comment and its failure text named columns that stopped naming the dev admin at #47). ⛔ No file undersrc/flows/, no object, no hook, noDESIGN.md. F10 is untouched — it does exactly what card 09 built it to do.First: the card's premise, re-measured on
mainThe card's reading was taken on
claude/issue-47-seed-legal-owner. Re-taken here onorigin/main@2d63324, clean database,pnpm demoon port 3152, README operator setup performed, re-seeded so the upserts hand the rows over, one trigger of each of the six daily jobs throughPOST /api/v1/automation/NAME/trigger, receipts read from the driver's own SQLite file (node:sqlite, read-only):The premise holds, exactly as written. 200 of 200, and F10's two are the only receipts in the whole book still addressed to the dev admin now that #26 and #47 have landed.
The deal: route A for the book, route B for one row
owneris optional, so it may name accounts that do not exist yet. It now names them by what the obligation is:complianceLegal Counsel 1/2, 29 / 27clm_legalRCU onclm_obligation, the only non-admin set that may create one, and all four seeded compliance titles are legal work: an insurance certificate, a sanctions re-screen, a data-transfer safeguard confirmation, an anti-bribery statementdeliverable·reportBusiness Requester 1/2/3, 50 / 53 / 40clm_requesterRU(本人负责), and §05 puts 我负责的履约 in the 我的合同 group every employee reaches. A delivery and a quarterly report are the launching desk's own workBoth halves deal by the counterparty relationship — the identical rule
ownerOfandlegalOwnerOfalready use, and structure rather than prose, sodemo-enanddemo-zhdeal identically. It names no new account: all five are already inREADME.md's exact-name table, which #55 built for exactly this. The table's two rows gain what they now also receive; no sixth name, and the other five positions stay "named however you like".owner_idDESIGN.md §03 declines to default this column to the contract owner, and
_daily-sweep.tsrepeats that permission where it explains whyhasRecipientexists. Putting all 200 on the launching requester would assert by construction the very thing §03 refuses to assert by default — and a reader could then no longer tell from the data that the two columns are independent at all. Dealing the compliance third away from the business owner is what makes that independence visible, and it is whyobligation_metrics(§09's third dataset, whose dimensions are status · kind · owner · contract) reads as five bars instead of one.Nothing in the product assigns this column. F9 creates renewal obligations from the type's defaults and leaves the owner to a person; S5's
extract_obligationsproposes a 负责人 that a person confirms (§07). So the deal states a rule a legal desk would recognise rather than pretending to replay one.What F10's "nobody to tell" edge still exercises — one row, and it is proved reachable
UNASSIGNED_OBLIGATIONS = 1, andleaveUnassignedinplan-children.tsproves F10 will select it. One, because one is what coverage costs — and one is therefore also the maximum, since every row beyond it is a reminder the demo does not send, which is the defect this PR exists to fix. The constant is a one-line change and the proof re-runs for whatever number it is given.Why the row's due date is constructed at T+7 rather than drawn — measured on
main, oneobligation_duerun on a freshly seeded database:The two zeroes are structural, not a bad day. Every stage filters
status IN (pending, in_progress); the plan draws every such row's due date from1 + rng*29or35 + rng*460, so no open obligation is ever due today (the minimum is +1) or already past due (the seeded arrears are bornoverdue, which no stage selects). Theweekstage's one-day[T+7, T+8)window is the only one that can carry this edge on this corpus, and how many rows land in it is otherwise a draw.The row is taken from the
due_soon_pendingband —pending, due in[1, 29]— so moving it inside that band leaves §10's "40 due in the next 30 days" exactly where it was. Taking afuturerow (+35 and out) would have quietly made it 41.deliverableis preferred for the story, not the mechanics: an extracted commitment nobody has been made accountable for yet is precisely the state §07's S5 leaves behind between "AI proposes a 负责人" and "a person confirms".leaveUnassignedre-proves both columns after the rewrite and throws with the reason if either moves; the eligibility check throws separately if the due-soon band ever empties.Selected vs notified, before and after — read from the running system
Identical protocol both sides: clean
.objectstack/data,pnpm demoon port 3152, README operator setup (3 requesters · all 7 positions · dev admin intoclm_admin), re-seed so the upserts hand the rows over, then one trigger of each job. Counts from each run's own node summary;failed=0on every run, both sides.Receipts by topic, both sides. This is the measurement the card asks for, and it is complete on both runs:
clm_legal_review_over_30_daysclm_obligation_due_soonclm_payment_overdueclm_turn_stalledStage census. Taken node by node from each run's own summary. The after side covers all six jobs; on the before side only F10's was taken directly, which is the one this card moves:
obligation_due(F10) week · today · arrearslegal_review_sla(F3) over 30d · 60dturn_stalled(F4)payment_overdue(F11) due · arrearsrenewal_notice(F12)expiration_sweep(F13)F10's
3 → 2is the whole change: three rows selected, two notified, one quiet by design — andfailed=0proves the sweep did not die on the unowned row, which is the edge being kept alive. F12's zero is unchanged and untouched (#47 ruled it correct and out of scope).Where the notifications land
45 receipts both sides. Zero now carry the dev admin — the last such receipt in the book is gone. Distinct recipients falls 7 → 6 because the dev admin drops out, which is the point rather than a regression: it holds no
clm_*permission set, so it was the one recipient that could not act.Driven in a browser
Chromium (
/opt/pw-browsers/chromium-1194/chrome-linux/chrome), against the same running demo, signed in as each account and clicked through to 我的合同 › My Obligations:my_obligationsrowsLegal Counsel 2Compliance, soonest first, oneOverdue)Business Requester 3Dev AdminGET /api/v1/data/clm_obligation?...&filter=[["owner","equals","9srl9g9N…"],["status","in",["pending","in_progress","overdue"]]] -> 200, 14 records, grid footer reads14 records. The administrator's own obligation queue is now empty, which is the fix stated as a screenshot.The one action the reminder asks for was driven too:
Legal Counsel 2PATCHed one of its own obligations toin_progressand got200. The same PATCH asBusiness Requester 3is refused — that refusal is not this card and is not caused by it; see Acceptance notes.Invariants
en820 ·zh-CN820, per-object row counts identical (120 · 60 · 25 · 30 · 200 · 300 · 40 · 9 · 30 · 6)clm_obligationcompared positionally across the two compiled artifacts: owner mismatches = 0, structural (kind/status/due/completed_at) mismatches = 0owner = NULLand every row survives — 820 rows, none refusedlegal_owneruntouchedcounselForextraction is a pure refactor and this is the check on itGates
Each redirected to a file with
$?read immediately after, no pipe between the command and the exit code.lint's 21 warnings and 5 suggestions are not new: the same two gates were run on a detached worktree at2d63324and the warning set diffs clean against this branch's — the only differing line is the config path. No changeset (this repo has no changeset gate).Control-character self-scan over the five changed files:
grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'— clean.Acceptance notes
Filed as #64 — DESIGN.md §04 gives
clm_requesterU(本人负责)onclm_obligation;controlled_by_parentrefuses it for all 200.Business Requester 3owns an obligation and its parent contract,POST /api/v1/security/explainanswersallowed: true, and the PATCH is still403 PERMISSION_DENIED — requires edit access to its master record.clm_requester's contract edit window is['draft','submitted']and every obligation hangs off an executed contract, so the two windows are disjoint by construction. ⛔ Not introduced here and not dodged here: the wall stands whoever owns the row, so re-dealing the obligations away from the business desk to avoid it would be widening a consumer around a defect that lives upstream. Legal is unaffected and was measured working.Already filed as #11 — the three business requesters do not get
clm_requesterby existing. An account with no position resolvesmember_defaultonly, andexplain read clm_obligationanswersallowed: false; thesys_user_permission_setgrant thatrequester.profile.tsnames as "the platform's supported path" is what turns it on (measured both ways). Every measurement above was taken with that grant applied. This is #11's decision, untouched here.Noted, not filed — only three of
clm_obligation.kind's six option values are ever seeded:deliverable72 ·report72 ·compliance56, whilepayment,renewalandothernever occur.planObligationsrotateskindonce per round-robin round over 72 parents, so 200 rows reach exactly three rounds. Nothing declares otherwise — §10 pins only the 200, the 40 and the 10 — so it breaks no contract, but it is why this deal could not use the card's own suggestion of "the finance controller for the payment ones": there are no payment obligations to give anyone. Whoever next touches the §10 obligation spread or bands a dashboard onkindwill meet it; there is no queued card that does.Noted, not filed — a non-admin console session logs
GET /api/v1/data/sys_activity -> 403 PERMISSION_DENIEDon the home page. Platform surface, nothing to do withclm_*; not patched (AGENTS.md "Platform gaps"). No queued card picks it up.Merged
origin/main(b667fda) — and what that does to the numbers aboveBranched at
2d63324;mainmoved twice while this was in flight.scripts/demo.mjsconflicted in the two passages #54 / PR #60 (6e534689) also edited — theDEMO_USERdoc block and theDev Adminname-check failure detail. Both edits move the same way: #60 droppedclm_contract.legal_ownerfrom the two reference lists, and this branch takes the same lists one reference further, toclm_review.revieweralone — the only onerequired: trueforces to resolve at seed time. Resolved to this branch's wording in both hunks, on top of #60, merge commit not rebase, no force-push.README.mdmerged with no conflict. PR #62 (#59) also landed; it touched onlysrc/dashboards/and the two bundles.Every measurement in this PR body still describes the merged tree, and that is checked rather than assumed. Nothing that merged in touches
src/data/,src/flows/or the objects, so the receipt census, the stage census and the browser readings stand as taken. Re-run after the merge:Both locales recompiled from the merged tree: 820 rows each, per-object counts identical,
clm_obligationpositionally identical (owner mismatches 0, structural mismatches 0), and the deal unchanged — 53 / 50 / 40 / 29 / 27 and one deliberately unassigned.node --check scripts/demo.mjsclean,OPERATOR_SETUP_NOTEitem 1 present exactly once, control-character scan clean.⛔ No re-seed and no re-measure were run, because nothing merged in could have moved them.
REWORK round — one sentence in
README.md(7645414)README.md:121claimedclm_review.reviewerwas "the only user reference in the fixture that still points at the dev admin". That is a universal, andclm_deviation.decided_bykills it:deviation.object.tsdeclares itField.user, andnegotiation.seed.ts:79still stamps itDEMO_USERon every decided deviation. Two references name the account; the sentence claimed one. Confirmed both readings against the tree rather than taking them on trust.Scoped to what the paragraph supports and to what the operator has to act on:
That claim is checkable, and it was checked: of the six user references in the model —
submitted_by,owner_id,legal_owner,decided_by,clm_obligation.owner,reviewer— onlyreviewercarriesrequired: true. The mechanism and the reassign instruction are unchanged.decided_byis deliberately left unnamed rather than listed beside it: it is an audit stamp, nothing is routed to it, there is nothing for the operator to reassign, and #61 is queued to re-deal it — naming it here would cost that card a second README edit. Omitting a member is incompleteness; the defect was the universal, not the omission. ⛔decided_byitself is untouched — that is #61's, deliberately queued to adopt this card's route as precedent.scripts/demo.mjs's "the ONLY reference this boot has to satisfy" is correct as it stands and is untouched: it is scoped to what the priming boot must resolve, anddecided_byis optional.Gates re-run on this commit:
pnpm validate→ 0 (✓ Validation passed (966ms)),pnpm lint→ 0 (21 warning(s), 5 suggestion(s)— the same 21),pnpm typecheck→ 0,pnpm lint:i18n-gate→ 0 (0 missing keys across 2 locale(s)). Control-character scan onREADME.md: clean.⛔ No re-seed, no re-measure, no browser run. A prose-only change to
README.mdcannot move a fixture measurement — the receipt census, stage census, locale-identity check and browser readings above are the ones taken earlier in this PR and are unchanged, not re-verified against this commit.Generated by Claude Code