Skip to content

feat(review): add bounded review contract surface - #378

Draft
seonghobae wants to merge 72 commits into
mainfrom
feat/review-contract-374
Draft

feat(review): add bounded review contract surface#378
seonghobae wants to merge 72 commits into
mainfrom
feat/review-contract-374

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Scope

Refs #374. This Draft is the canonical single-writer lane for Inkspan's provider-neutral review contract and controlled presentation surface. It currently includes deterministic target/suggestion validation, revision-bound operation evidence, bounded host-supplied thread presentation metadata, a controlled accessible React adapter, deterministic thread-list keyboard traversal, fail-closed collection ingress, public typed-error normalization for untrusted host presentation/intent failures, a controlled accessible inline target marker, and a fail-closed exact-revision admission guard for suggestion mutations.

It does not claim the full #374 product slice is complete or shipped. Protected main remains the only shipped implementation authority.

Lifecycle truth

Do not treat any PR-body SHA, workflow result, merge base, or review inventory as durable lifecycle truth. Before every source write, protected-main reconciliation, review transition, or merge decision, independently refetch the submitted head, protected main, ancestry/mergeability, active writers, formal reviews/threads, live rulesets, and all applicable exact-head repository/central workflows. The PR API's embedded base.sha may be historical and is not a substitute for resolving the live protected-main tip.

Current active-PR surface

The Draft exports a React-free @contextualwisdomlab/cwl-editor/review package subpath plus optional @contextualwisdomlab/cwl-editor/review-react presentation. Review evidence uses Inkspan's canonical document revision and W3C TextPositionSelector projection instead of DOM offsets. Inputs are bounded and validated; snapshots are detached/frozen where applicable. Inkspan does not acquire host identity, authorization, persistence, tenancy, transport, durable audit, retention, collaboration admission, credential, network, or model-policy authority.

CwlReviewThreadList validates collection shape before item inspection, rejects oversized/sparse/non-enumerable/accessor-backed slots fail-closed, validates host labels/action callbacks, supplies accessible thread/action names, and maintains one deterministic roving thread target. Arrow Up/Down/Home/End move DOM focus only and do not commit host selection. Private exceptions from validated host label/intent callbacks are normalized to the public CwlReviewPresentationError contract rather than exposing private host details.

CwlReviewTargetMarker exposes the active review target as a bounded accessible button with deterministic selected-state semantics and controlled host intent callbacks; invalid labels/callbacks fail closed through the same public presentation-error boundary rather than acquiring editor-selection authority.

assertReviewSuggestionCurrentRevision is the mutation-boundary admission guard for provider-neutral insert/delete proposals. It validates the untrusted suggestion, computes the exact current document revision through the canonical revision implementation, and fails closed with the stable redacted stale_operation contract when the target revision differs. It returns only the detached validated suggestion and does not copy the current document body into review evidence. Hosts retain authorization, persistence, conflict policy, and durable audit authority.

TDD lineage

The callback-failure work established that React/JSDOM error-report cardinality is not a product contract. The durable invariants are that the host callback executes once, externally observed failures use the stable public CwlReviewPresentationError contract, and private host exception details are not exposed.

The stale-revision mutation guard was developed test-first on this existing branch: a RED contract first required suggestions targeting another document revision to be rejected before mutation, followed by the narrow implementation that computes the canonical current revision and returns stale_operation on mismatch. Repository CI for lifecycle decisions must always be re-fetched at the exact current head rather than inherited from this lineage.

The inline target marker was likewise developed test-first on this branch: a test-only parent commit required accessible target intent before the narrow presentation implementation landed. Exact-current-head repository CI is the only GREEN evidence for lifecycle decisions.

Shared control-plane evidence

Central workflow evidence is owned by the organization .github boundary. Known aggregate-green false-evidence classes include dependency-review jobs that skip the actual review action and scanners that analyze a synthetic merge rather than the submitted head. Existing owner paths are ContextualWisdomLab/.github#810 for dependency-review fail-closed behavior and ContextualWisdomLab/.github#1222 for exact-source scanner binding. Do not add Inkspan-local competing workflow writers or transfer predecessor/synthetic-source evidence.

Pending, queued, skipped, cancelled, absent, neutral, failed, stale, predecessor, status-only, model-only, wrong-checkout, or synthetic-source-only evidence is non-passing.

Remaining #374 work

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

  • deterministic ProseMirror accept/reject/rollback adapters for insertion/deletion suggestions with undo/redo proof;
  • review-specific real-browser/keyboard/screen-reader journeys;
  • explicit print/export review-markup policy;
  • executable canonical contract/ADR/UML/data-model/threat-model/traceability updates as semantics settle;
  • Storybook or equivalent buyer-visible states/visual regression evidence;
  • packed-artifact consumer acceptance for each newly exposed review API.

Integration boundary

Keep this PR Draft while #118 owns stable-release publication/provenance/digest acceptance and until the branch is reconciled with live protected main through a safe, non-destructive, source-owner-aware path. Do not force-push/destructively rebase, self-approve, weaken gates, transfer predecessor evidence, fabricate release identity, or represent Draft behavior as protected-main shipped truth.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

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

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

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head local validation for 721061d: the ten changed review and React review-contract files passed, 47/47 tests. Expected jsdom stderr from fail-closed rendering cases was contained by the passing assertions. The PR remains Draft; this is focused local evidence only, not approval or merge evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: api API, protocol, event, or external contract priority: medium Normal-priority or P2 work status: draft Draft pull request type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): add provider-neutral comments, suggestions, and revision review surface

1 participant