Skip to content

Commit a7870ef

Browse files
committed
yama: add soundness/polarity lens + 2 blocking criteria (review follow-up to #56)
Applies the review feedback from PR #56 that merged before it could be folded in. The Yama reviewer was steered toward 'no Obj.magic/@unwrap/%identity' but not toward the subtler class of bug that every mechanical gate (golden compile, bench) passes green on — an unsound type that compiles. That's exactly the #55 regression a human had to catch (an output-only 'a from an `any` getter return). - workflowInstructions: a 'compilation is NOT soundness' lens (CI proves compiles + matches baseline, not soundness; a fake behind a shared typeRef doesn't bubble to the report) and a PRODUCE/RECEIVE polarity rule ('a is sound only in a consumer-supplied input position). - focusAreas: an all-cases-or-flag line (every union/overload variant must be reachable or the whole prop is flagged — never silently dropped). - blockingCriteria (+2 -> 6): output-only 'a that doesn't round-trip; a fake emitted UNFLAGGED inside a shared *Types.res entry. Config-only; doesn't touch the generator.
1 parent 119f847 commit a7870ef

1 file changed

Lines changed: 27 additions & 0 deletions

File tree

yama.config.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,22 @@ review:
8484
order, Date, or filesystem ordering leaking into output), and whether an
8585
intentional output change silently regresses previously clean bindings.
8686
87+
CRITICAL LENS — compilation is NOT soundness. The CI gates (npm test, golden
88+
compile, bench) prove the output COMPILES and MATCHES the committed baseline; they
89+
do NOT prove the emitted types are sound. An unsound `'a` external always compiles;
90+
a fake `string` hidden inside a shared *Types.res entry compiles and may not even
91+
appear in the report (imperfection bucketing stops at a typeRef boundary, so a fake
92+
behind a shared type reference does not bubble to the prop). Judge each `'a`, each
93+
@unboxed / views-module arm, and each shared-type field by REASONING about runtime
94+
values and data-flow direction — never by whether it builds.
95+
96+
POLARITY — for every emitted type ask who PRODUCES the value: the consumer (INPUT —
97+
a prop, a method/constructor param, the return of a callback the consumer implements)
98+
or the library (OUTPUT — a getter/method return, or a callback PARAM the consumer
99+
receives). A type variable `'a` is sound ONLY when it appears in a consumer-SUPPLIED
100+
input (it round-trips); an `any`/unmodellable type in an OUTPUT position must stay a
101+
flagged placeholder, never a free `'a` (which unifies with anything).
102+
87103
Be specific and constructive: every comment cites the exact file+line, the
88104
concrete risk, and a suggested fix. Match the terse, heavily-commented style of
89105
src/*.mjs. Only block per <blocking-criteria>; otherwise leave inline comments
@@ -108,6 +124,11 @@ review:
108124
round-tripping generic; multi-type props → @unboxed untagged variant
109125
(distinct runtime tags) or an opaque module; `any` is a defect, never
110126
silently typed.
127+
- All-cases-or-flag: every TS variant of a union / overload must be
128+
reachable in the emitted output, or the WHOLE prop is flagged — never
129+
silently drop one (e.g. two string literals colliding on one constructor
130+
ident like `'trap-focus'`/`'trapFocus'`, or a dropped overload arm, both
131+
still compile).
111132
- name: "Determinism & reproducibility"
112133
priority: "CRITICAL"
113134
description: |
@@ -157,6 +178,12 @@ review:
157178
- condition: "Generated ReScript introduces an unsafe cast that violates the contract — Obj.magic, @unwrap, or a bare %identity outside the two documented allowances (opaque-module from*/as* accessor, render-prop <prop>Fn wrapper)"
158179
action: "BLOCK"
159180
reason: "Breaks the core 'no unsafe casts' guarantee in docs/TYPE_MAPPING.md / CLAUDE.md"
181+
- condition: "Generated ReScript introduces a type variable ('a) that does NOT round-trip — it appears only in OUTPUT positions (a getter/method return, or a callback param the consumer receives) and never in a consumer-supplied input, so it unifies with anything (e.g. an `any` field surfacing as `routes(): RouterRoute<'a>[]`)"
182+
action: "BLOCK"
183+
reason: "An output-only 'a is an unsound cast (rule #4: a type var is only for a genuine round-tripping generic) — strictly worse than a flagged placeholder, and it compiles, so no other gate catches it"
184+
- condition: "A fake / plausible-but-wrong type (e.g. `string` where the TS type was richer) is emitted UNFLAGGED inside a shared *Types.res entry (an @unboxed arm, a record field, or a views-module member) — no ⚪/🔍/🛑 bucket comment, because the prop only holds a typeRef and the imperfection does not bubble up"
185+
action: "BLOCK"
186+
reason: "'Flag, don't fake' must hold even when the fake hides behind a shared type reference and the report shows the prop as usable"
160187
- condition: "A change makes generated output nondeterministic (same input → differing output across runs) — e.g. unordered iteration, time, or randomness reaching emitted code"
161188
action: "BLOCK"
162189
reason: "Determinism is a core product guarantee; nondeterministic output is unshippable"

0 commit comments

Comments
 (0)