Skip to content

Product close-out: record design decisions as product defaults, add CI - #6

Closed
baozhoutao wants to merge 1 commit into
mainfrom
claude/issue-3-product-closeout
Closed

baozhoutao wants to merge 1 commit into
mainfrom
claude/issue-3-product-closeout

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Work item #3. Documentation and CI configuration only — no code change.

The maintainer ruling of 2026-09-02 takes this system forward as a standard product: every open decision closes along the AI-recommended path rather than waiting for item-by-item sign-off. This PR records that as documentation truth and gives the repo a CI workflow, so main and every PR run the same gate that already exists locally as pnpm verify.

What changed

docs/03-方案分级.md — each of the three multi-path proposals (D-01 organisation subject, D-02 scoring engine, D-03 review state machine) now ends in a 放行记录 line reading 「按推荐路径执行(产品默认)」 with the 2026-09-02 ruling date and the executed path. The 执行说明 paragraph and the section heading no longer describe the decisions as pending, and each 待确认点 has become a resolved 产品默认 statement. The trailing global record is replaced by a 决策台账 closing note, so exactly three 放行记录 lines exist.

docs/01-需求解读报告.md — one 「产品默认:…」 line appended to Q-01Q-05, Q-07Q-09 and Q-11, each naming the adopted option and why. Q-06, Q-10 and Q-12 remain open and are untouched.

docs/04-需求符合度清单.md — rows 6, 10, 11 and 12 now cite the product default instead of a pending question; row 12 in particular loses 「疑点 Q-04 待拍板」. No verdict (✅/⚠️/❌) changes.

README.md — the document table no longer calls the decisions pending, and a new 「产品默认口径」 section tabulates all twelve defaults (D-01~D-03 plus the nine questions) and names the three questions that remain open.

.github/workflows/ci.yml — new. Triggers on push to main and on pull_request. Node 22 and pnpm 10 to match the engines field, pnpm install --frozen-lockfile, then pnpm validate / pnpm typecheck / pnpm test as three separate steps so a red run names the failing gate without opening the log. Workflow CI, job verify, so the check reads CI / verify.

src/, scripts/, test/ and package.json are untouched, as the work item requires.

Evidence

Acceptance criterion 4 — no hit, and the ruling vocabulary is gone from the README too:

$ grep -rn "待放行" docs/ ; echo "exit=$?"
exit=1

$ grep -rn "待拍板" docs/ README.md ; echo "exit=$?"
exit=1

Exactly three release records in docs/03, all in the target wording:

$ grep -n "放行记录" docs/03-方案分级.md
64:*放行记录:按推荐路径执行(产品默认),维护者 2026-09-02 拍板:待决策项按 AI 推荐处理。执行路径 A(复用平台 `sys_business_unit`)。*
94:*放行记录:按推荐路径执行(产品默认),维护者 2026-09-02 拍板:待决策项按 AI 推荐处理。执行路径 A(服务端计分引擎 + before 写入 hook)。*
125:*放行记录:按推荐路径执行(产品默认),维护者 2026-09-02 拍板:待决策项按 AI 推荐处理。执行路径 A(自建审核状态机)。*

Nine product-default lines in docs/01, on exactly the nine closed questions:

$ awk '/^\*\*Q-[0-9]+/{q=$0} /^\*\*产品默认\*\*/{print substr(q,1,8)}' docs/01-需求解读报告.md
**Q-01
**Q-02
**Q-03
**Q-04
**Q-05
**Q-07
**Q-08
**Q-09
**Q-11

The workflow parses, and the check name resolves as intended:

$ python3 -c "import yaml; d=yaml.safe_load(open('.github/workflows/ci.yml')); ..."
parsed OK
workflow name : CI
triggers      : {"push": {"branches": ["main"]}, "pull_request": null}
jobs          : ['verify']
check will be : CI / verify

Local gate at a9ab27e, the same command the workflow runs:

$ pnpm verify ; echo "VERIFY EXIT=$?"
  ✓ Validation passed (750ms)
  Data: 17 Objects  182 Fields
> tsc --noEmit
 Test Files  5 passed (5)
      Tests  55 passed (55)
VERIFY EXIT=0

Diff scope, confined to the five declared files:

$ git diff --stat 19cde36..HEAD
 .github/workflows/ci.yml          | 47 ++++++++++++++
 README.md                         | 28 ++++++++-
 docs/01-需求解读报告.md            | 18 +++++++
 docs/03-方案分级.md                | 33 ++++++-----
 docs/04-需求符合度清单.md          |  8 ++--
 5 files changed, 118 insertions(+), 16 deletions(-)

Baseline differences from the work item description

Two sub-items of the work item were already in their target state on main, landed by #2, and were therefore left alone rather than re-applied:

  • docs/04 row 7 is already ✅ and row 18 already describes the dynamic record sharing;
  • the README role table already carries the 「private + 动态记录共享」 wording in its data-scope column.

The statistics line was consequently not re-synced: this PR changes no verdict, so nothing in the counts moved. One pre-existing inconsistency is noted for triage rather than fixed here — the statistics line counts 21 ✅ / 3 ⚠️, which treats row 4 (xlsx direct import) as ⚠️ in line with the exit paragraph below it, while row 4's own 态 cell reads ✅. Recounting the 态 column alone gives 22 ✅ / 2 ⚠️. Reconciling the two would flip a compliance verdict, which is outside this work item.

Verification of CI / verify itself can only happen once GitHub runs the workflow on this PR; its conclusion is reported on the work item.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb


Generated by Claude Code

The maintainer ruling of 2026-09-02 takes this system forward as a standard
product: every open decision closes along the AI-recommended path. Record that
as documentation truth and give the repo a CI workflow so `main` and every PR
run the same gate that already exists locally.

- docs/03: a 放行记录 line under each of D-01 / D-02 / D-03 reading
  「按推荐路径执行(产品默认)」 with the ruling date; the 执行说明 paragraph and
  the section heading no longer describe the decisions as pending, and each
  待确认点 becomes a resolved 产品默认 statement.
- docs/01: one 「产品默认」 line on Q-01~Q-05, Q-07~Q-09 and Q-11 giving the
  adopted option and why. Q-06, Q-10 and Q-12 stay open and untouched.
- docs/04: rows 6, 10, 11 and 12 now cite the product default instead of a
  pending question. Rows 7 and 18 and the statistics line already carry their
  target wording from #2, so they are left as they are.
- README: the doc table no longer calls the decisions pending, and a new
  「产品默认口径」 section lists all twelve defaults plus the three questions
  that remain open.
- .github/workflows/ci.yml: new. push to main + pull_request, Node 22 and
  pnpm 10 to match `engines`, `pnpm install --frozen-lockfile` then validate,
  typecheck and test as separate steps so a red run names the failing gate.
  Job name `verify`, so the check reads `CI / verify`.

No code change: src/, scripts/, test/ and package.json are untouched.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown

os-project-dev-review — PR #6

Tier: light (docs-only + a new CI workflow; no runtime code in the diff)
Verdict: NOT MERGEABLE — 1 blocking, 5 should-fix findings below.

Reviewed cold against work item #3, main@19cde36..a9ab27e. Every self-reported ✅ was re-run locally; every newly asserted "产品默认" was checked against the code it now describes as settled.


General quality

.github/workflows/ci.yml is correct as written, and I found no defect in it:

  • pnpm is installed before actions/setup-node, which cache: pnpm requires — the ordering most hand-written pnpm workflows get wrong is right here.
  • package.json has no packageManager field, so pnpm/action-setup@v4 with version: 10 will not hit the "Multiple versions of pnpm specified" failure. pnpm 10 / Node 22 satisfy engines (pnpm >=10.15, node >=22).
  • pnpm-lock.yaml is committed, so --frozen-lockfile holds.
  • Job id and name: are both verify, so the check-run name matches the acceptance criterion literally; the three steps are exactly package.json's verify script (validate && typecheck && test), and the inline comment saying so is accurate.
  • concurrency + cancel-in-progress, permissions: contents: read, and timeout-minutes: 15 are all sensible for a verify-only workflow.

Triggers match the work item (push: [main] + pull_request). Check run verify (id 100249781200) on a9ab27e is success.

The docs are well written — each "产品默认" line gives a reason rather than just a letter, and the reasons are mostly good ones (Q-08's "flow-per-department would move the flow definition from the plan to the subject, which fights per-version freezing" is exactly the right argument). The problem is not the prose; it is that five of the newly asserted defaults describe behavior the code does not have, and docs/04 now reports them as implemented.


Library-specific checks

① Change-surface / no out-of-scope edits — PASS.
Exactly the five expected paths: .github/workflows/ci.yml, README.md, docs/01, docs/03, docs/04. Nothing under src/**, scripts/**, test/**; package.json untouched, as specified. No revert of #2/#4: README.md's role table 数据范围 column still reads private + 动态记录共享 for all three scoped roles (the README hunks are only the doc index and the new section), and docs/04 #7 (✅, BonusAfterDecideHook) and #18 (dynamic sharing rules, "不再依赖企业版 hierarchy-security") are context-only in the diff.

② Self-report reconciliation — MIXED.
All four self-reported ✅ reproduce:

  1. docs/03 lines 64 / 94 / 125 — "放行记录:按推荐路径执行(产品默认),维护者 2026-09-02 拍板…", each naming the executed path. ✅
  2. Nine **产品默认** lines in docs/01 at 118/123/128/133/138/146/151/156/164. I checked every option letter against the work item: Q-01 A+B+C+D, Q-02 目标÷实际, Q-03 A, Q-04 不封顶不保底 + 人力审核提出/人力负责人审批, Q-05 A, Q-07 退回上一节点 + 全部分公司重核, Q-08 B, Q-09 B, Q-11 A — all nine match verbatim. ✅
  3. Check run verify = success on the head SHA. ✅
  4. grep -rn "待放行\|待拍板" docs/ README.md → no hits. ✅

The ⚠️ note ("docs/04 #7/#18 and the README data-scope column were already achieved by #2 on the baseline, not re-touched") is accurate — I traced the docs/04 statistics line through history (4649470 19→20, a4b5354 20→21, dropping #7 and #18 from the ⚠️ list), so requirement field 3 including "统计行同步" is genuinely satisfied by the baseline.

What the self-report does not mention is the reconciliation that matters most for a closeout PR: whether the code backs the defaults now being asserted as truth. For Q-03, Q-04 and Q-09 it does not — see findings.

③ Forbidden-trace check — PASS (none).
Single commit, no src/** / scripts/** / test/** edits, no release-notes file, no config / permission-set / hook changes, no stash residue, no unrelated files.

④ Ruling landed in user-visible copy — PASS (one nit).
The new README "产品默认口径" section is business language: every table row states the rule in domain terms (计分方式 / 完成率 / 得分合成 / 驳回口径 / 版本并存), and the three still-open questions are named in plain words rather than left as bare ids. D-01D-03 and Q-01Q-11 are document-local ids defined in the two linked docs, not internal codenames. Nit only: "不再逐项等人表态" is internal-process framing in the most customer-visible file; "AI 推荐路径" is at least consistent with the vocabulary docs/03 already uses.

Extra checks requested.

  • docs/03: no residual "待放行 / 待技术负责人放行" describing D-01~D-03 as open. All three **待确认点** blocks are gone (git grep 待确认点 → 0 hits). The single "未经放行" occurrence at line 9 is inside a negation ("不再标注「未经放行、可撤回」") and reads correctly.
  • docs/01: Q-06 / Q-10 / Q-12 are byte-identical to main, carry no 产品默认 line, and README names exactly those three as still awaiting the customer. ✅

Findings

🔴 docs/04:16 asserts a 口径 the code does not implement (Q-03). The row's 偏差与出口 column previously said, honestly, "口径细节…按推断实现,待疑点 Q-01Q-05 拍板后只改数据 / 一处代码". It now says "口径按产品默认(Q-01Q-05…)实现". For Q-03 that is not true. README.md:86 and docs/01:128 both state the personal score as 板块得分 × 分工权重 × 个人系数 + 承接项得分 × 承接权重, but src/lib/aggregate.ts:97 computes the second term from the line's 得分率 (%), not its 得分: round2((rate * it.weight) / 100). For a carried indicator held at 20% sheet weight and scored 100%, the sheet line is worth 20 points; at 承接权重 50% the documented formula gives 10 and the code gives 50 — a 5× divergence on a salary-linked number. aggregate.ts's own header says so explicitly ("承接项得分率…(疑点 Q-03 未拍板,按推断口径实现)"). src/** is out of scope for this PR, which is exactly why the compliance checklist must not claim conformance: keep the deviation in #6's 偏差与出口 column (and open a follow-up issue to bring aggregate.ts to the ruled formula, or amend the ruling), rather than converting a truthful caveat into a false ✅.

🟡 Q-09's guarantee is unenforced, and docs/04:25 now marks it ✅ as a product default. "同周期只有一个生效版本" is not checked anywhere: PlanPublishHook validates steps, subjects, indicator weights, targets and open disputes only, and src/objects/plan.object.ts's sole period validation is plan_period_order (period_end >= period_start). Publishing a second draft over the same period_type/period_no succeeds and produces a second entry sheet per subject plus a second kpi_result set — precisely the "这个人这个周期得几分没有唯一答案" the rationale says the default prevents. Either record it as a deviation or file the follow-up before calling it settled.

🟡 Q-04 names an approval chain that is not segregated — and it is now the only control. docs/01:133 / README.md:87 say "提出人为人力审核,审批人为人力负责人". src/hooks/bonus.hook.ts:30 accepts kpi_hr_reviewer or kpi_hr_head for the approve/reject stamp, and src/security/index.ts:45 grants kpi_hr_reviewer_set kpi_bonus: editOrg (= allowCreate + allowEdit, org scope). A user holding only 人力审核 can raise a bonus and approve it on the next save. This matters more after this PR than before it: the same paragraph removes the cap and floor and says "需要边界时用审批而不是用公式来把关" — so the doc leans the whole control onto a separation of duties that does not exist.

🟡 Q-02's stated reason is inverted (docs/01:123). The default picks A over B because B would "给出无上界的完成率". It is the other way round: B (1 − (实际−目标)/目标 = 2 − 实际/目标) is bounded above by 200% as 实际→0, while A (目标 ÷ 实际) is the unbounded one — computeCompletionRate returns t/a*100 and only short-circuits at actual <= 0, so actual=0.01 against target=100 yields 1,000,000%. linear/step/range clamp the score rate afterwards, but the completion rate itself is what the doc is arguing about, and for formula indicators that raw rate is handed to CEL unclamped. Choice A may well still be right (symmetry, no per-indicator formulas); the justification just argues for the opposite conclusion, in a file labelled "可直接转发客户".

🟡 docs/01:77 (R-高-01) still contradicts the closeout, in the very file this PR edits. It reads "计分与到人折算口径未定" and recommends "按疑点 Q-01~Q-05 逐条拍板" — the thing this PR just did, twenty lines above the new 产品默认 rows. A reader hits the open risk first. Refresh the 建议动作 to point at the defaults (and keep the residual risk, which is real: the defaults still need checking against the customer's Excel).

🟡 CI pins action majors that GitHub has already deprecated. The green run's own annotation says: "Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4, actions/setup-node@v4, pnpm/action-setup@v4." For a workflow being added today, actions/checkout@v5 and actions/setup-node@v5 avoid a warning on every run and the eventual hard break. (pnpm/action-setup has no v5 yet — leave it.)

⚪ Q-07's reset does not cover the re-submit path (src/hooks/sheet.hook.ts:173, out of scope — follow-up only). The task reset is guarded by action === 'reject' inside the toStatus === 'branch_checking' block, so it fires when a rejection lands on the check node. When a branch checker rejects, the sheet goes to draft; on re-submit action is 'submit', the creation loop skips branches that already have a task, and nothing resets. Net effect: B1 confirms, B2 rejects, the department fixes the figure and re-submits, B2 confirms — and CheckTaskAfterDecideHook auto-advances with B1's confirmation still standing against data it never saw. The doc's stronger reading ("数据一改,已确认的分公司确认的就不再是同一份数据") is the correct behavior; the code covers only half of it.

⚪ src/lib/aggregate.ts:6 is now stale — "疑点 Q-03 未拍板,按推断口径实现" and "(疑点 Q-05 推断口径)" sit in the file the new README calls the 唯一真值. Out of scope here (src/** is off-limits); worth carrying in the same follow-up as the 🔴.

⚪ docs/03:55 keeps a superseded rationale next to the new one. Option A's 优点 cell still says "数据范围(unit / unit_and_below)原生按它解析" (and :48 "深度按组织树解析"), while the 产品默认 paragraph added two lines below correctly says the scope now resolves through dynamic per-plan sharing rules. Pre-existing, and an alternatives table is legitimately a record of what was known at decision time — but the contradiction is newly adjacent.

⚪ docs/02:9 still reads "审定记录 | 待技术负责人审定". Outside the work item's file list and about blueprint sign-off rather than D-01~D-03, so not a scope violation; flagging only because a reader who takes the README's "不再逐项等人表态" at face value will trip over it.

⚪ docs/04:53 counts #4 in the ⚠️ bucket while its 态 column is ✅ (21+3 = 24 items, but the 态 column reads 22 ✅ / 2 ⚠️). Pre-existing since the file's first commit and untouched by this PR; noting it only because the work item named the statistics line.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

On hold — not merged. Maintainer ruling (2026-09-02): the design proposal (docs/00-设计方案.md, PR #8) must be confirmed before further development. The review findings on this PR (Q-03 personal-item term, Q-09 single-active-version enforcement, Q-04 approval segregation, Q-02 rationale, R-高-01 wording, action majors) all touch口径 that the proposal puts up for confirmation as items 4–7, 10 and 12; they will be resolved together with that confirmation, either by adjusting the code or by amending the recorded defaults. Rework is dispatched only after the proposal is confirmed.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Closing as superseded. The "product default" framing this PR recorded has been replaced by the customer's written confirmation of the design proposal (docs/00, all 17 items confirmed on 2026-09-02; items 13/14/15 = A). The documentation baseline and CI workflow are re-specified in a new work item that starts from the current main; the ci.yml draft here is reused there with updated action majors.


Generated by Claude Code

@baozhoutao
baozhoutao deleted the claude/issue-3-product-closeout branch September 3, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants