Skip to content

feat(email-writing): define strict review contracts - #1327

Draft
seonghobae wants to merge 11 commits into
feat/inkspan-email-writing-guidefrom
feat/llm-email-writing-contracts-task2
Draft

feat(email-writing): define strict review contracts#1327
seonghobae wants to merge 11 commits into
feat/inkspan-email-writing-guidefrom
feat/llm-email-writing-contracts-task2

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Scope

This Draft PR implements Task 2 only of the email-writing guidance plan: strict backend/frontend review contracts and deterministic transport/integrity validation. It does not add persistence, authorized thread construction, contextual-orchestrator calls, Candidate/Judge semantics, review API, editor integration, sending, policy publication, or release behavior.

Semantic authority remains strict: these contracts may validate JSON shape/size, Unicode, revision/hash, selector bounds and transport safety, but must not infer grammar, clarity, tone, pragmatics, technical suitability, actionability, or intent through keyword/regex/phrase-list/sender-domain/recipient-count/language-name/nearest-text/position rules.

Live stack state — 2026-09-01

Parent #1322 is 9f1836d09e6b4db97855d701d8220268e9fb4d87. Task 2 is now ab74a345e4d03680da32a7eac2cc1fca3005cfb3 and contains that exact parent as a non-destructive merge parent. The PR base snapshot resolves to the same root head.

The unique Task-2 delta remains the nine focused contract/test files: .github/workflows/email-writing-contracts-tdd.yml, backend/services/email_writing_contracts.py, its focused backend tests, and the frontend email-writing transport/parser implementation and tests. The design-root documentation is inherited from #1322 and is not Task-2-owned semantic work.

All predecessor-head checks/reviews are stale. Fresh exact-head evidence must be evaluated only for ab74a345e4d03680da32a7eac2cc1fca3005cfb3.

Contract behavior retained

  • exact SHA-256 document revision and strong entity-tag validation;
  • W3C TextPositionSelector transport using Unicode-code-point offsets;
  • bounded JSON bytes/nodes/depth/arrays/objects/strings/diagnostics;
  • duplicate-key, malformed UTF-8, non-scalar Unicode, non-finite number, extra-field, coercion and unsafe-identifier rejection;
  • privacy-minimized provenance without prompt, source-mail, draft, replacement/explanation, credential or provider-trace plaintext;
  • explicit incremental/deep selector rules and applied-feedback revision rules;
  • conversion to Inkspan diagnostics without adding semantic inference.

No mutable Inkspan branch/package is consumed here. Runtime/editor consumption waits for an immutable released Inkspan artifact exposing the required writing-diagnostics public contract with integrity and source provenance.

Dependency continuation

#1328 has been reconciled onto this exact Task-2 head as ea61b9de9cd26a26209da36e858510a31486531a; later task-owned descendants have likewise been advanced in dependency order. No predecessor evidence transfers across head movements.

Evidence and merge boundary

Keep this PR Draft while fresh current-head gates and review/thread state are regenerated. Queued/pending/skipped-required/cancelled/neutral/failed/absent/stale/predecessor/synthetic/model-only/status-only/author-only evidence is non-passing. Merge requires the unchanged exact head to satisfy live protected-branch/ruleset requirements, all applicable exact-head CI/security/coverage/package/provenance gates, zero valid unresolved findings, and the qualifying independent approval required by live governance after the last push.

There is still no shipped writing-guidance feature in this PR; the existing editor and send path remain available.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 58 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 3e5bd40f-6330-4fc2-8f3d-a6d15c13271d

📥 Commits

Reviewing files that changed from the base of the PR and between 9f1836d and ab74a34.

📒 Files selected for processing (9)
  • .github/workflows/email-writing-contracts-tdd.yml
  • backend/services/email_writing_contracts.py
  • backend/tests/test_email_writing_contracts.py
  • backend/tests/test_email_writing_contracts_coverage.py
  • backend/tests/test_email_writing_contracts_terminal_coverage.py
  • frontend/src/lib/email-writing.coverage.test.ts
  • frontend/src/lib/email-writing.parser-coverage.test.ts
  • frontend/src/lib/email-writing.test.ts
  • frontend/src/lib/email-writing.ts

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.

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for ab74a345e4d03680da32a7eac2cc1fca3005cfb3:

  • Draft PR: merge automation is paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review the unchanged exact current head fb7c406ee1328a6ac42dbaf54bb6852c199d8b0a (normal merge of live parent #1322 bfc2df11). Predecessor c5fec3e5 evidence does not transfer. Local Task 2: backend 13 passed / 100% coverage, frontend 19 passed / 100% coverage. Keep Draft. This comment is not an approval.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

@seonghobae I will review the current head fb7c406ee1328a6ac42dbaf54bb6852c199d8b0a. I will keep the PR in Draft. This request does not approve or merge the PR.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cursor Bot pushed a commit that referenced this pull request Aug 17, 2026
…ask4' into feat/llm-email-writing-orchestrator-task5

Retarget Task 5 onto live #1329 head 4570747
(merged onto live #1328 51fb5e8 / #1327 fb7c406 / #1322 bfc2df1 / develop@dd8d1519).

Preserve the hardened contextual-orchestrator boundary. Do not restore
write-capable Task 5 promotion/finalize workflows.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@opencode-agent opencode-agent Bot added 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 labels Aug 22, 2026
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.


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.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current head c3147918fd75af2092171e8e6d404057fd0659d3 against exact parent #1322 ec0e1d367a0fb03f4431286260897e010421ef0c. The focused backend-contracts and frontend-contracts checks on this head are terminal-success; predecessor reviews/checks do not transfer. Please verify the strict transport/integrity contract, no-lexical-semantic-fallback boundary, Unicode-code-point selector semantics, bounded parsing, privacy-minimized provenance, and fail-closed malformed-input behavior. This request is for findings only and is not approval.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='c3147918fd75af2092171e8e6d404057fd0659d3'
parent='ec0e1d367a0fb03f4431286260897e010421ef0c'

echo '== Commit topology =='
git rev-parse "$head" "$parent"
git merge-base --is-ancestor "$parent" "$head" && echo 'stated parent is an ancestor'
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
echo

echo '== Changed paths against stated parent =='
git diff --name-status "$parent" "$head"
echo
git diff --stat "$parent" "$head"
echo

echo '== Production file outlines =='
ast-grep outline backend/services/email_writing_contracts.py --items all || true
ast-grep outline frontend/src/lib/email-writing.ts --items all || true
echo

echo '== Backend implementation =='
cat -n backend/services/email_writing_contracts.py
echo

echo '== Frontend implementation =='
cat -n frontend/src/lib/email-writing.ts
echo

echo '== Targeted boundary searches =='
rg -n -i -C 2 \
  '(keyword|phrase|grammar|clarity|tone|semantic|language|recipient|sender|domain|nearest|regex|revision|sha-?256|etag|TextPositionSelector|code.?point|utf-?8|duplicate|finite|provenance|diagnostic)' \
  backend/services/email_writing_contracts.py frontend/src/lib/email-writing.ts || true
echo

echo '== Test outlines and focused assertions =='
for f in \
  backend/tests/test_email_writing_contracts.py \
  backend/tests/test_email_writing_contracts_coverage.py \
  backend/tests/test_email_writing_contracts_terminal_coverage.py \
  frontend/src/lib/email-writing.test.ts \
  frontend/src/lib/email-writing.coverage.test.ts \
  frontend/src/lib/email-writing.parser-coverage.test.ts
do
  echo "-- $f --"
  ast-grep outline "$f" --items all || true
  rg -n -i -C 1 \
    '(unicode|code.?point|surrogate|duplicate|utf-?8|non.?finite|extra|coerc|bound|depth|node|provenance|privacy|selector|revision|etag|malformed)' \
    "$f" || true
done

Length of output: 110339


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Frontend raw-parser bound tests =='
sed -n '360,430p' frontend/src/lib/email-writing.coverage.test.ts
sed -n '1,120p' frontend/src/lib/email-writing.parser-coverage.test.ts
echo

echo '== Searches for total-node accounting and non-BMP length coverage =='
rg -n -i -C 2 \
  '(MAX_JSON_NODES|node_limit|node.?count|code.?point|codePointAt|Array\.from|spread\(|emoji|non.?BMP|surrogate pair|astral)' \
  frontend/src/lib/email-writing.ts \
  frontend/src/lib/email-writing.test.ts \
  frontend/src/lib/email-writing.coverage.test.ts \
  frontend/src/lib/email-writing.parser-coverage.test.ts \
  backend/tests/test_email_writing_contracts.py \
  backend/tests/test_email_writing_contracts_coverage.py || true
echo

echo '== Exact frontend implementation lines for candidate findings =='
sed -n '151,171p;380,515p' frontend/src/lib/email-writing.ts
echo

echo '== Working tree and exact diff context =='
git status --short
git diff --unified=20 ec0e1d367a0fb03f4431286260897e010421ef0c c3147918fd75af2092171e8e6d404057fd0659d3 -- frontend/src/lib/email-writing.ts

Length of output: 33467


@seonghobae I reviewed c3147918fd75af2092171e8e6d404057fd0659d3 against ec0e1d367a0fb03f4431286260897e010421ef0c. The stated parent is an ancestor. The diff contains the nine stated files.

I found two contract issues.

  1. High — Enforce the total JSON-node limit in the frontend parser.
    backend/services/email_writing_contracts.py rejects more than 20_000 nodes before model validation. frontend/src/lib/email-writing.ts only limits nesting and direct object or array members. A payload under 1_000_000 bytes can contain more than 20_000 nodes while each container has at most 1_000 members. The frontend constructs that value and reaches schema validation instead of failing at the bounded parsing boundary. Add a parser-wide node counter with the same 20_000 limit. Add a test that produces more than 20_000 nodes without exceeding the depth, direct-member, or byte limits.
    Affected code: frontend/src/lib/email-writing.ts, Line 393 through Line 462.

  2. Medium — Count Unicode scalars, not UTF-16 code units, for string limits.
    unicodeString uses value.length for empty and maximum-length checks. In JavaScript, String.length counts UTF-16 code units. A valid non-BMP scalar uses two code units. Therefore, the frontend rejects a value of 4_000 non-BMP Unicode scalars for fields that have a 4_000 limit, while the Python backend accepts it because Python len counts Unicode scalars. This causes backend/frontend contract divergence for valid Unicode transport content. Count Unicode code points before applying each maximum. Keep the existing malformed-surrogate rejection. Add boundary tests with non-BMP Unicode.
    Affected code: frontend/src/lib/email-writing.ts, Line 157 through Line 160.

I did not approve or merge this Draft PR.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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 enhancement New feature or request 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.

2 participants