feat(review): add bounded review contract surface

[seonghobae]

Scope

Refs #374. This Draft is the canonical single-writer lane for Inkspan's provider-neutral review contract and controlled presentation surface. It includes deterministic target validation, bounded insert/delete proposal metadata, revision-bound accept/reject evidence, bounded host-supplied thread presentation metadata, a controlled accessible React adapter, deterministic keyboard traversal among thread-selection targets, fail-closed collection-ingress preflight, and public typed-error normalization for host semantic-summary localization failures. It does not claim the full inline-marker/suggestion UI, editor transaction adapter, durable review workflow, or shipped review product is complete.

Protected shipped truth remains exact main@3b38ead2d00f44eb578d0689087b9293b3dabe1e. The current exact Draft head is eb91c8ff945b9f6dac0a3b9edd10433313969eea; the immediate pre-write refetch reports the Draft mechanically mergeable against that protected base. Mergeability is not authorization.

Current active-PR surface

This Draft exports a React-free @contextualwisdomlab/cwl-editor/review package subpath plus optional @contextualwisdomlab/cwl-editor/review-react presentation. The review core uses Inkspan's canonical document revision and W3C TextPositionSelector evidence rather than DOM offsets. Targets, suggestions, operation results, and thread presentation snapshots are strictly validated, detached/bounded where applicable, and do not become host authorization, persistence, identity, tenancy, transport, or durable-audit state.

CwlReviewThreadList validates the collection before rendering and then validates every presentation item. The collection is capped at 1,024 entries before any item inspection. Every slot must be a dense, enumerable own data property; accessor-backed, sparse, or non-enumerable slots fail closed. An accessor-backed slot's getter is not invoked by collection entry extraction. Each accepted data value still passes through the React-free review presentation validator, and duplicate validated threadKey values are rejected. The 1,024 value is a private adapter render-work bound, not an external protocol/schema limit. This contract intentionally does not claim that JavaScript Proxy reflection traps cannot execute.

The component also treats actor/label/action inputs as untrusted host presentation data. Current behavior covers host actor labels with whitespace normalization, optional timestamps, custom renderActor, native submit proposal actions, read-only rendering, hostile React-node/accessor/prototype inputs, and deterministic keyboard traversal. ArrowDown, ArrowUp, Home, and End move DOM focus only among thread-selection targets; traversal does not commit host-controlled selection or invoke onSelectThread. Native activation remains the selection boundary. Semantic-summary label callbacks are normalized through the public CwlReviewPresentationError boundary rather than leaking private host-localization exceptions. The component emits presentation/action intent only; the host remains admission/transport/authorization authority.

Standalone product boundaries remain intact: no service, database, credential, network, model, host identity, tenancy, or durable persistence authority is introduced. Model/provider output remains untrusted proposal data.

Exact TDD / failure lineage

The keyboard behavior was added test-first on this existing owner branch:

• RED commit 2fb8a12c192f38194c646e164ae2abd187f66e09 added a realistic two-thread traversal contract. Exact CI run 32601503553 failed in build/test job 97100353882 before the production change, while Security and SAST remained terminal success.
• Intermediate test hardening ed0fe008facc1fae051f76fb25aaed6acda6e01b added a neutral-key control so non-navigation keys do not move focus.
• GREEN commit 7423f6ca69dfa16afaaaeb9c8bc5b4b42bc81867 added the narrow focus-only implementation in src/review-react/index.tsx without adding persistence, transport, authorization, or selection authority.

The collection-ingress behavior was then repaired test-first on this same owner branch:

• RED commit 987c27692bfd8e164b1d99e65c14351bad9fe68f added realistic contracts for accessor-backed entries and an oversized collection. Exact CI run 32613146473 failed in build/test job 97129296916 on exactly the two new assertions while 863 existing tests passed. This falsified the prior collection-preflight claim: an accessor-backed entry could be evaluated before validation, and an oversized array was not rejected before entry inspection.
• GREEN commit 771c156d1eb787908e75dfcaea9a9d6c1c8fc39c added the narrow collection preflight and extended the contract tests to sparse and non-enumerable entries. No host getter is used to extract an array-slot value, and the bound is checked before slot inspection.

A later exact-head CI failure exposed a test-contract contradiction in the semantic-summary failure case rather than a production behavior defect:

• Predecessor head d0ebe3c52c854d9e7ed93b199b24366f9f13e60d failed CI run 32632930281, build/test job 97178334460, because semanticSummary.test.tsx compared a newly constructed generic Error while production deliberately normalized private host callback failures to the public CwlReviewPresentationError contract.
• The smallest causal repair is current head eb91c8ff945b9f6dac0a3b9edd10433313969eea: the test now asserts the exported CwlReviewPresentationError type. Production error normalization was not weakened or bypassed.

All workflow evidence attached to predecessor heads, including prior 7423f6..., 987c276..., 771c156..., and d0ebe3... runs, is lineage only and is non-passing for the current head.

Exact-current evidence

For exact head eb91c8ff945b9f6dac0a3b9edd10433313969eea against protected base 3b38ead2d00f44eb578d0689087b9293b3dabe1e at the immediate pre-write refetch:

• CI 32638108233: completed / success.
  • build-and-test/package 97190787031: completed / success; steps include checkout, frozen-lock install, typecheck, test with 100% coverage, library build, packed-package consumer verification, and demo build.
  • Cross-engine Clipboard / Playwright 97190786861: completed / success.
  • Office Python 3.11 97190787017, 3.12 97190786985, 3.13 97190787058, and 3.14 97190787032: completed / success, including 100% docstring coverage, 100% branch coverage, wheel build, and packaged schema/license verification.
• Security Scan 32638108228: completed / success.
• SAST Semgrep 32638108250: completed / success.
• formal submitted reviews: 0;
• unresolved inline review threads: 0;
• Draft: yes; mechanically mergeable: yes.

Pending, queued, in-progress, skipped, cancelled, absent, neutral, failed, stale, predecessor, status-only, model-only, or wrong-head evidence is non-passing. Repository checks do not substitute for separately applicable central workflows, qualifying review, or then-live governance. The generic cross-engine repository job is not represented as proof of all review-specific browser or screen-reader scenarios that remain in #374.

Ownership boundary

Inkspan owns deterministic review target/suggestion validation, canonical revision/selector evidence, revision-only operation classification, bounded provider-neutral presentation metadata, controlled accessible presentation, collection-ingress workload/preflight semantics, focus-only keyboard navigation semantics, and normalization of presentation callback failures to its public presentation-error contract in this slice. Hosts continue to own durable comment/thread/suggestion IDs and bodies/records, actor identity, authentication/authorization, tenancy, persistence, retention/legal hold, notifications, durable audit, cross-revision re-anchoring/conflict policy, collaboration-provider admission/room authorization, credentials, transport, model policy, and network services.

Remaining #374 work

This PR does not close #374. Remaining work includes:

• full inline target-marker interaction and editor-selection coupling;
• richer comment/suggestion interaction plus deterministic ProseMirror editor-transaction adapters for insertion/deletion accept/reject/rollback and undo/redo behavior;
• stale-revision fail-closed integration proof at the mutation boundary;
• review-specific keyboard/browser flows and screen-reader semantics beyond the now-covered deterministic thread-list traversal;
• explicit print/export review-markup policy;
• canonical contract/ADR/UML/data-model/threat-model/traceability updates as executable semantics settle;
• Storybook or equivalent buyer-visible states/visual regression evidence;
• packed-artifact and consumer acceptance for any newly exposed review API as scope evolves.

Integration boundary

Keep this PR Draft while #118 owns the frozen v0.6.0 publication/provenance/digest boundary. Before any lifecycle transition, refetch exact head/live base, ancestry/mergeability, live governance, formal reviews/threads, and all applicable exact-head repository/central workflows. Do not self-approve, weaken gates, transfer predecessor evidence, fabricate release identity, or represent Draft behavior as protected-main shipped truth.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}