fix(docs): state the sheet comment anchor limitation instead of implying precision

[211-lee]

Closes #1182

Summary

Sheet comments anchor to an absolute cell coordinate and nothing re-maps that
coordinate when rows or columns are inserted or removed, so a comment can end up
pointing at a different cell. The drift is undetectable client-side (see below),
so this PR states the limitation in the panel instead of implying a precision we
do not deliver. One caption, one i18n key per locale, no CSS rule added.

The actual fix — re-mapping anchors, and offering the user a "re-point this
comment / make it a whole-sheet comment" escape hatch — needs the backend to
allow editing an existing comment's anchor and is tracked separately in #1182's
out-of-scope section.

Linked Spec

Issue #1182. The evidence for "undetectable" is in the code: parseCell
(SheetCommentPanel.tsx:39-55) decodes ${sheetId}!${row}:${col}, and
anchorText is written as ref.a1 (:390) — an A1 label, not a content
snapshot. A drifted coordinate therefore parses cleanly, resolves to a populated
cell, and carries no referent to compare against.

How verified

• npx vitest run src/sheet/ → 117 passed (10 files), incl. the 14 existing
  SheetCommentPanel.test.ts specs, matching the pre-change baseline
• npx vitest run src/i18n/i18n.test.ts → 5 passed (this is the locale parity
  guard: it flattens both trees and diffs the key sets)
• pnpm -w run i18n:check → 0 candidates within baseline; locale keys healthy
• npx tsc --noEmit → byte-identical to the recorded baseline (3941 lines, all
  pre-existing dmworkbase react@17 noise; zero under packages/docs/src/)
• Key presence checked directly in both JSON trees rather than inferred — the
  parity test only catches a key present on one side, so a symmetric omission
  would pass every gate and surface as a raw key string at runtime

Not verified by screenshot: pure copy plus an existing style class, no new CSS
rule. Say so and I will attach light/dark captures.

COMPREHENSION

What this does to the load-bearing path. Nothing. No anchor is read, written,
encoded, or decoded differently. parseCell, cellMatches, createRoot, the
badge painter in SheetView.tsx:228-237, and the marker click handler are all
untouched. The change is one conditionally-rendered <p> between the header row
and the composer, plus one leaf key in each locale file.

What it could break. Three things were considered. (1) Layout — the caption
sits inside .octo-comment-panel as a normal block, not in the .octo-member-row
flex line, so it cannot displace the title, the resolved toggle, or the close
button. (2) Noise — gating on threads.length > 0 keeps it off the empty state,
which already renders its own .octo-comment-empty line; the two never appear
together. (3) CSS collision — packages/docs/src/editor/styles.css is the
shared stylesheet the sheet imports and is under concurrent edit on another
branch, so this PR adds no rule there at all; it reuses .octo-comment-empty
(which tracks --octo-muted, so both themes follow) and expresses the one bit of
spacing as an inline style, matching existing precedent in the same file.

What verified it. The full sheet suite against a recorded baseline, the locale
parity test, i18n:check, and a byte-level typecheck diff. The claim I am
least able to verify mechanically is the product judgement — that an
unconditional caption is better than a conditional badge. The argument is in the
code comment: a partial badge would imply the unbadged threads were verified,
which is the same false assurance in a new shape. If the product view differs, the
gate is one && and the copy is one key.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

Code Review — PR #1184 (octo-web)

Summary

This PR adds an informational notice to the sheet comment panel (SheetCommentPanel.tsx) warning users that sheet comments anchor to absolute cell coordinates (${sheetId}!${row}:${col}) and may drift after row/column insertions or deletions. The notice is rendered conditionally when threads.length > 0 (i.e., only when there are comments to read), reusing the existing .octo-comment-empty muted-caption style. Two i18n locales (en-US, zh-CN) gain a new anchorNotice key under docs.sheet.comment.

The change is small, well-scoped, and correctly documents a known limitation rather than implying a precision the product does not deliver.

Verification

• ✅ i18n key parity — docs.sheet.comment.anchorNotice present in both en-US.json:582 and zh-CN.json:582; the component resolves it via t('docs.sheet.comment.anchorNotice') following the same dot-notation pattern as sibling keys (docs.sheet.comment.menu, docs.sheet.comment.placeholder, etc.).
• ✅ CSS class defined — .octo-comment-empty at packages/docs/src/editor/styles.css:2743 sets color: var(--octo-muted, #86909c); font-size: 13px;. The inline style={{ margin: '0 0 4px' }} overrides only margin, inheriting color and font-size correctly.
• ✅ Data flow — threads is destructured from comments (prop type UseDocComments) at SheetCommentPanel.tsx:316. The threads.length > 0 guard correctly suppresses the notice on an empty comment list.
• ✅ Comment accuracy — parseCell at SheetCommentPanel.tsx:39 parses base64-encoded ${sheetId}!${row}:${col} coordinates. No re-mapping logic exists for row/column insertions/deletions. The comment block accurately describes this limitation.
• ✅ No dangling references — no other locale files exist beyond en-US and zh-CN; no orphan keys.

Findings

No P0/P1 issues. No correctness, security, build, or functional concerns.

Verdict: APPROVED

Straightforward documentation of a known product limitation. The notice is conditionally rendered, correctly localized, and reuses existing styling. No defects found.

[Jerry-Xin]

Accurate, well-scoped copy fix: it states the sheet-comment anchoring limitation instead of implying a precision the feature does not deliver. Verified at head a3f9fbd.

Scope — pure copy + a conditional caption, no behavior change

• The three files are: two locale files (en-US.json, zh-CN.json) adding one string, and SheetCommentPanel.tsx rendering it. Anchor parsing, cell matching, creation, and navigation are untouched — confirmed by diff. The "docs" label matches reality; there is no hidden behavior change.

i18n consistency

• New key sheet.comment.anchorNotice is added to BOTH locales with matching nesting and no key drift (full top-level key sets are identical between en-US and zh-CN). Both JSON files parse cleanly.
• The call site t('docs.sheet.comment.anchorNotice') follows the package's established docs.-prefix convention (identical to the sibling t('docs.sheet.comment.placeholder') in the same file, which resolves against the same locale block). No dangling key, no hardcoded string bypassing i18n.

Render / safety

• Guarded by threads.length > 0, so it never collides with the empty-state caption. Rendered as JSX text ({t(...)}), so it is auto-escaped — no XSS surface.
• Reuses the existing .octo-comment-empty class, which is defined and tracks --octo-muted for correct light/dark theming — the code comment's claim checks out.

Non-blocking

• The inline style={{ margin: '0 0 4px' }} is a literal spacing value. This package (@octo/base docs editor) consistently uses literal-px inline spacing and --octo-* vars rather than the --wk-sp-* design tokens (the --wk-* token rule in DEVELOPMENT.md targets the dmworkbase design system, not this package). The change matches the adjacent line 408 style={{ flex: 1, margin: 0 }} and ~12 sibling inline-spacing usages across this package, so it is consistent with local convention. Optional: if the team wants to converge, a shared caption class would work — but this is not required for this PR.

Highlights: honest, self-documenting comment explaining why an unconditional notice (rather than a per-comment "stale" badge) is the correct call; locale parity is clean; zero behavior risk.

[yujiawei]

Code Review — PR #1184 (octo-web)

Reviewed at head a3f9fbdf, against merge-base b5f38b9f. 3 files, +21/−2.

1. Spec compliance

Spec: ✅

Checked against the linked issue #1182, which asks for: an unconditional caption in the sheet comment panel stating the anchor limitation, rendered only when comments exist, explicitly not a per-thread conditional badge, and explicitly not the anchor re-mapping fix (backend-gated, out of scope there).

• Missing: none. The caption is present (SheetCommentPanel.tsx:431-435), gated on threads.length > 0 as specified, and the zh-CN copy is verbatim from the issue.
• Extra: none. Nothing outside the caption and one leaf key per locale. The anchor read/write path is untouched — parseCell (:39-55), cellMatches (:67-69), the createRoot call (:390), and the grid badge painter (SheetView.tsx:228-237) are all byte-identical to base.
• Divergence: none. The PR does not attempt the out-of-scope re-mapping, and does not implement the per-thread badge the issue rules out.

Things I verified rather than took on trust:

• The i18n key resolves. The key nests as sheet.comment.anchorNotice in both locale trees, while the call site reads t('docs.sheet.comment.anchorNotice'). The docs. segment is the namespace registered in module.tsx:259, matching the sibling t('docs.sheet.comment.placeholder') two lines below — so this is correct, not an off-by-one path.
• Locale parity holds. Both JSON trees carry the key. I checked this by flattening both files directly, not by trusting the parity test, since a symmetric omission passes that test.
• The gate is right in every state. threads is the same array the <ul> maps (:497) and the empty state keys off (:495), so the caption and docs.comment.empty can never render together, and during initial load (threads empty) the caption is correctly absent.
• Layout does not collapse. .octo-member-row carries padding: 6px 0 (styles.css:1531-1536), so the caption's margin-top: 0 still leaves 6px below the header rather than butting against it.

2. Code quality

Quality: Approved — no P0/P1. Five P2s, none of which should hold the merge.

[P2] The new branch has no test coverage. SheetCommentPanel.test.ts:96 — makeComments() defaults to threads: [], and all six render specs use that default, so every existing test exercises threads.length > 0 === false. The reported "117 passed, matching the pre-change baseline" is therefore accurate but not informative about the added JSX: no test renders it. The infrastructure is already here — renderPanel(sheet, makeComments({ threads: [thread] })) plus a getByText / queryByText pair would lock both sides of the gate in about six lines. Worth adding, since the gate is the one piece of logic this PR introduces.

[P2] .octo-comment-empty now means two things. SheetCommentPanel.tsx:432 reuses the empty-state class for a caption that only appears when the list is not empty. Today this is harmless — the rule sets only color and font-size (styles.css:2743-2746), and I checked the two open PRs that touch that stylesheet; neither modifies this rule, so there is no live collision. But the coupling is real: any future empty-state-specific rule added there (centering, min-height, illustration padding) silently restyles this caption. The trade-off is defensible — the PR avoids styles.css to stay clear of a concurrent edit — but it buys merge safety with a naming hazard, and that debt should be paid down (dedicated class, or an alias applied alongside) next time that file is free.

[P2] The caption scrolls out of view on long threads. .octo-doc-drawer is the scroll container (overflow-y: auto, styles.css:1914-1932) and the panel is its only child, so the caption scrolls away with the header. To be clear about the mechanism: DOM order is correct — the caption at :431 precedes the thread list at :497 — this is purely about scroll position. A user with twenty threads reads most of them below the fold with no caveat on screen. position: sticky; top: 0 is the fix and it needs a styles.css rule, which this PR deliberately avoids; pairing it with the class rename above is the natural follow-up.

[P2] The copy enumerates a narrower cause than the mechanism has. The anchor is an absolute coordinate, so anything that relocates content relative to coordinates drifts it — not only row/column insert and delete, but also a cut/paste of a range or a fill-drag. A user who reads "After rows or columns are inserted or removed" and did neither of those may conclude their comments are still accurate. The trailing "verify before relying on it" partially covers the gap, which is why this is P2 and not blocking. Widening the phrasing (for example "…inserted or removed, or cell contents moved…") is one string per locale and no code change, and it closes the same class of false assurance this PR exists to remove.

[P2, nit] No semantics on the caption. It is a bare <p>. For a caveat whose whole job is to qualify the trustworthiness of the content below it, role="note" is one attribute.

On hard-coded spacing (style={{ margin: '0 0 4px' }}): I considered flagging this against the spacing-token rule and decided it does not apply here. That rule governs the --wk-* themed surface; this package runs its own --octo-* scheme with literal pixel values throughout (styles.css:2730-2746), the same component hard-codes style={{ flex: 1, margin: 0 }} eight lines above, and SheetView.tsx:125-135 hard-codes colors inline in the same feature. Enforcing tokens on this one line would be inconsistent, not stricter.

3. Verdict

APPROVE. The change is scoped exactly to what the linked issue asks for, it does not touch the anchor path, and the reasoning for an unconditional caption over a conditional badge is sound: with anchorText holding an A1 label rather than a content snapshot, there is genuinely no referent to compare against, and a partial badge would relocate the false assurance rather than remove it. The P2s are follow-up material.

I also want to name what the PR gets right beyond the diff: it is explicit that the product judgement is the part it cannot verify mechanically, and it puts that argument in a code comment where the next reader will find it. That is the right place for it.

4. Suggested follow-ups

1. Add the two-case render test for the threads.length > 0 gate (highest value of the five — it is the only logic here).
2. When styles.css is next free of concurrent edits: give the caption its own class and make it position: sticky; top: 0 in the drawer. These two are one change.
3. Widen the copy to cover content-moving operations, not just row/column structural edits.

5. Additional observations (outside this PR's scope)

• The inline composer never shows the caveat. A comment can be created entirely from the floating cell composer (SheetView.tsx:784, SheetCommentComposer at :100) without ever opening the panel, so an author can create a comment having never seen the limitation. The linked issue scopes the notice to the panel, so this is not a gap in the PR — but the caveat arguably matters most at authoring time, and that surface is currently uncovered.
• The underlying data-integrity gap (annotations bound to mutable coordinates rather than stable cell identities) remains, as the PR states. It is tracked in the issue's out-of-scope section and correctly not attempted here.

6. What I did not verify

• I did not run the unit suite. node_modules is not installed in my checkout, and CI has no unit-test job for this path — install-build, Build, and e2e-p0 are green, which corroborates the typecheck claim but not the vitest run. The "117 passed" figure is the author's report, unverified by me. My locale-parity and key-resolution checks above are independent of it.
• No visual verification, in either theme. The caption inherits --octo-muted, so theme-following is sound by construction, but the wrapped height of the ~200-character English string inside a 360px drawer is unmeasured. The author's offer of light/dark captures is worth taking up if that width concerns you.
• I reasoned P2-4's cut/paste and fill-drag vectors from the anchor model, not from an inventory of which operations the sheets-core preset actually exposes. The row/column case is confirmed; treat the others as likely rather than proven.

[lml2468]

Review + Verdict: #1184 — fix(docs): state the sheet comment anchor limitation instead of implying precision @ a3f9fbdf

锚定 head a3f9fbdf4e75(rev-parse == live head)/ blocked。3 文件 +21/−2,纯文案 + 一处条件性提示,零行为变更。逐处审阅 + 实测。

结论:APPROVE ✅(可合并)—— 诚实的能力边界声明:表格评论锚点在行列增删后可能漂移,PR 如实提示而非暗示不具备的精度;i18n 双语一致、key 就位、CI 全绿、无注入面。

规格符合 ✅

• 意图:表格评论锚定绝对单元格坐标(${sheetId}!${row}:${col}),行列插入/删除后无重映射 → 评论可能指向别的单元格,且因 anchorText 存 A1 标签(非内容快照)无法检测漂移。PR 选择无条件如实声明该限制,而非做一个「看起来陈旧」的徽标(此处不可实现,且会暗示未加徽标的都已核实)。文案与代码事实一致——正是本 PR 目标。

代码质量 ✅(逐处核实)

• 零行为变更:diff 仅 3 文件;锚点 parse/match/create/navigate 未触碰(scope 确认)。SheetCommentPanel 仅加一段解释性注释 + 条件 <p>。
• 条件渲染合理:{threads.length > 0 && <p>…</p>} —— 仅在有评论时显示,空列表不显示(避免纯噪声)。复用 .octo-comment-empty(styles.css:2743,muted 文字、追随 --octo-muted 主题)。
• i18n 一致 + key 就位:anchorNotice 加于两 locale 的 sheet.comment 下,与已工作的同级 placeholder/current/add/viewBadge 并排;调用点 t('docs.sheet.comment.anchorNotice') 与同文件已工作的 t('docs.sheet.comment.placeholder')(:445)同款约定,解析正确。i18n:check passed(0 candidates,locale keys healthy),无 dangling/漂移。
• 安全:文案作转义 JSX 文本渲染,无 innerHTML、无注入面。

验证(实测 @ a3f9fbdf)

• i18n:check ✅ locale keys healthy;frozen-install 干净。
• FULL CI 全绿:Build / install-build / e2e-p0 / dependency-review 系 / osv-scan / secret-scan / history / label / pr-title-lint 均 pass;check-sprint/satisfy-code-review/scan-scheduled skipping(流程门)。

🟡 非阻断(局部约定,非缺陷)

• 内联 style={{ margin: '0 0 4px' }}:--wk-sp-* token 规则治的是 dmworkbase,不覆盖本 @octo/base docs-editor 包——该包统一用字面 px 内联 + --octo-* 变量(邻近 style={{flex:1,margin:0}}、FormulaPicker 近似 margin:'0 0 4px 2px')。与局部约定一致,非违规。

---

明确判断:APPROVE(可合并)。 纯文案 + 条件提示:如实声明表格评论按行列位置锚定、增删行列后可能漂移需核对,不再暗示不具备的精度;锚点解析/匹配/创建/导航零变更;双语 i18n 一致且 key 与已工作同级并排、调用约定相同,转义文本无注入面,CI 全绿。内联 spacing 属该包既有约定,非阻断。感谢 @211-lee。