feat(diagnostics): add strict writing diagnostic contract - #249
feat(diagnostics): add strict writing diagnostic contract#249seonghobae wants to merge 26 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
opencode-agent
Bot
added
area: api
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export { | ||
| DEFAULT_WRITING_DIAGNOSTIC_LIMITS, | ||
| WritingDiagnosticError, | ||
| validateWritingDiagnostics, | ||
| } from '../writingDiagnostics.js'; |
There was a problem hiding this comment.
🟡 New writing-diagnostics subpath not published in package exports
This framework-independent public surface has no matching ./writing-diagnostics entry in package.json exports, no vite build config, and no verify script, unlike every sibling subpath. It is never emitted to dist, so consumers importing @contextualwisdomlab/cwl-editor/writing-diagnostics hit a module-resolution failure. The exports test passes only because it resolves source, not the packed artifact.
Prompt for agents
The new public subpath src/writing-diagnostics/index.ts is not wired into the package the way every other subpath is. To make `@contextualwisdomlab/cwl-editor/writing-diagnostics` resolvable from the published package, mirror the existing text-position-selector subpath setup: (1) add a `./writing-diagnostics` entry to the `exports` map in package.json pointing at the built dist files (types + import + require); (2) add a vite build config (e.g. vite.writing-diagnostics.config.ts) that builds src/writing-diagnostics/index.ts as a React-free library entry and emits its d.ts, and add it to the `build` script; (3) add a packed-artifact verification script (like scripts/verify-text-position-selector-subpath-package.mjs) and reference it from `verify:package`, per the AGENTS.md rule to validate public package behavior from packed artifacts rather than source imports alone.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified against current #249 head b878721bf1c085612f8aea889081f0322e62a2ae and the existing downstream package owner #282 head 03e626e57c9b85d5f6cc9b616249b09bdf1669e2.
#249 is intentionally Task 1's source-level, React-free contract. Its executable export test is explicitly named writing diagnostic public source exports and imports ./writing-diagnostics/index.js from source. Package publication is not owned by this branch.
Existing Draft #282 (feat(diagnostics): publish framework-neutral package subpath) is the sole package-surface writer. On its exact current head, package.json already contains ./writing-diagnostics types/import/require exports, adds vite.writing-diagnostics.config.ts to build, and adds verify-writing-diagnostics-subpath-package.mjs to verify:package; that PR also owns the packed-package verification/build files. Moving those package files into #249 would create a competing writer and break the accepted dependency split.
Therefore the reported module-resolution problem is valid as a downstream package-publication requirement, but it is not a defect in #249's bounded source-contract scope. Package resolvability remains acceptance work on #282 and cannot be claimed from #249 alone.
| let prototype: object | null; | ||
| let keys: PropertyKey[]; | ||
| try { | ||
| prototype = Object.getPrototypeOf(value); | ||
| keys = Reflect.ownKeys(value); | ||
| } catch { | ||
| throw new WritingDiagnosticError(errorCode); | ||
| } | ||
| if (prototype !== Object.prototype && prototype !== null) { | ||
| throw new WritingDiagnosticError(errorCode); | ||
| } |
There was a problem hiding this comment.
🔍 100% coverage gate requires every defensive branch to be exercised
The repo enforces 100% branch coverage across src/**. writingDiagnostics.ts adds many defensive branches (the prototype === null path in readExactObject, the try/catch redaction paths). Any single one left unexercised fails pnpm coverage. The focused tests appear thorough, but full branch coverage is worth confirming.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Scope
Stacked Task 1 implementation lane for PR #248's writing-diagnostics plan. The PR remains Draft and targets
design/llm-writing-diagnostics; #248 is the governing dependency-root authority.Current exact stack authority
design/llm-writing-diagnostics@f97d9d6787fd0d6c01bd245506e8a220af8a8a5a;e80545e6a85b628d624f43d60259f16c6b2244d6;The branch was synchronized non-destructively through a two-parent commit over the exact current #248 tree. Fresh comparison resolves merge base to exact parent, reports
behind_by=0, and leaves exactly this lane's seven paths changed. No force push or destructive rebase was used.Test-first behavior
The contract was developed RED→GREEN before production implementation and provides provider-neutral diagnostics/provenance types, bounded priorities/resources/strings/selectors, exact own-data-property validation, hostile-reflection rejection, duplicate-ID rejection, exact revision/projection binding, deeply detached/frozen accepted values, stable redacted failures, and root plus React-free exports.
It performs no semantic language judgment, keyword/regex/phrase fallback, model/provider/network/database/credential access, stale-text repair, decoration, document mutation or send gate.
Exact-current-head evidence
Current exact head
e80545e6a85b628d624f43d60259f16c6b2244d6has no qualifying current-head PR workflow runs returned by the live workflow-run query yet. Therefore predecessor focused TDD success is historical only and current-head CI/security/SAST/package/browser/coverage evidence remains non-passing until freshly produced. Formal submitted reviews and unresolved inline threads are currently zero.Remaining gates
main@3b38ead2d00f44eb578d0689087b9293b3dabe1e.Do not self-approve, transfer predecessor evidence, weaken gates, bypass dependency order or synthesize release identity.