Conversation
…odes
A complex value is authored as either rectangular (re/im) or polar
(magnitude/phase, in radians), discriminated structurally by which fields
are present rather than by a form tag -- z.discriminatedUnion cannot host
two members sharing one literal discriminant value ("complex" twice; it
throws lazily at parse time), so ComputedValueSchema and
ExpressionNodeSchema each become a plain z.union of their existing core
discriminated union alongside a further nested union of the two complex
shapes.
Restructuring ExpressionNodeSchema this way changes its top-level JSON
Schema conversion from a flat oneOf to a nested anyOf, so the two test
helpers that walk its discriminant kinds (one against the runtime
.options, one against the generated JSON Schema branches) now recurse
into a nested union instead of assuming every branch is a flat leaf.
Adds a complex case throughout the evaluator: compare/memberOf equality (eq/neq only -- complex numbers have no natural ordering), negate (via rectangular normalisation), and the full arithmetic dispatch -- add/subtract/multiply/divide over two complex operands or a mixed complex/number pair (a plain number widens to complex with a zero imaginary part), power restricted to a real dimensionless integer exponent computed by repeated multiplication, and modulo left undefined. Division by a zero-magnitude complex number is domain-error, the same category as ordinary division by zero.
…r out-of-scope bullet Adds a "Complex values" section covering the rectangular/polar dual-input design and why it needs a structural union rather than a form tag, the toRectangular normalisation (mirroring duration's own millisecond normalisation), the number-widens-to-complex mixed-arithmetic rule, and power/modulo/ordering-operator scope limits. Extends the ComputedValue and ExpressionNode type blocks, the compare section's operand-kinds sentence, and the indeterminacy reference table's compare row. Complex support moves this out of "Out of scope", leaving symbolic algebra as the only remaining delegation precedent there.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Pushed 4 commits addressing the adversarial review findings on this branch: Fixed (real defects):
All of lint, typecheck, Note for whoever picks this up: this PR is closed as a duplicate of #8, which already implements the dimensionless-base-and-exponent power check and the modulo-as-domain-error classification correctly -- these fixes match what #8 already does independently. #8's |
Fixes #2
Adds a
complexcomputed value /complexLiteralexpression node, with arithmetic (add/subtract/multiply/divide/power), comparison (eq/neq), negation, and membership support in the evaluator.Schema design: a structural union, not a tag field
A complex number can be authored either rectangular (
re/im) or polar (magnitude/phase, in radians). The natural first instinct is a single object type with both sets of fields optional plus a runtime check that exactly one set is present -- but that throws away the type safety this package relies on everywhere else (no exhaustive narrowing, and the shipped JSON Schema would have to fall back to a looseanyOf-with-manual-validation shape instead of describing the real constraint).The alternative -- a discriminant tag (
form: "rectangular" | "polar") alongside a singlekind: "complex"-- doesn't work either, for a reason specific to this schema's construction:PredicateNodeSchema/ExpressionNodeSchemaandComputedValueSchemaarez.discriminatedUnion("kind", [...]), and Zod's discriminated union requires every member to have a distinct literal value for the discriminant key. Two members both carryingkind: "complex"is rejected (verified empirically -- it doesn't even get to a validation error, it throws lazily the first time something is parsed against it).So
complex/complexLiteralis a genuine two-branch union (ComplexRectangularSchema | ComplexPolarSchema, each az.strictObject), discriminated structurally by which fields are present (re/imvsmagnitude/phase) rather than by an extra tag. This meantComputedValueSchemaandExpressionNodeSchemathemselves could no longer stay flatz.discriminatedUnions -- each is now a plainz.unionof (the existing discriminated union of every other node kind) alongside (the nested complex union).PredicateNodeSchemais untouched; predicates don't carry computed values directly.This has one real consequence worth flagging for review:
ExpressionNodeSchema's top-level JSON Schema conversion changes from a flatoneOf(one entry per kind) to a nestedanyOf(two entries, one of which is itself aoneOf/anyOf). The two test helpers that walk the discriminant-kind set (one against the runtime.options, one against the generated JSON Schema) now recurse into nested unions instead of assuming every branch is a flat leaf object -- see the first commit. Everything downstream (the shippedschemas/trilean.schema.json, the smoke tests against the built dist/ output) still round-trips correctly; verified end to end.toRectangularnormalises either form to{ re, im }before any arithmetic runs, mirroring the existing precedent of normalising everydurationto milliseconds before combining two of different units.Scope
add/subtract/multiply/dividesupport twocomplexoperands or a mixedcomplex/numberpair (a plainnumberwidens tocomplexwith a zero imaginary part).poweragainst acomplexbase is defined only for a real, dimensionless, integer exponent (computed by repeated multiplication); a non-integer or complex exponent iswrong-type.modulois not defined forcomplexvalues.compare's ordering operators (gt/gte/lt/lte) arewrong-typeagainstcomplex-- there's no natural ordering -- leavingeq/neq, andmemberOfextends the same equality tocomplexcandidates.domain-error, same category as ordinary division by zero.README gets a new "Complex values" section, and the "Complex-number or phasor arithmetic" bullet is retired from "Out of scope".
Verification
pnpm lint && pnpm typecheck && pnpm test && pnpm test:integration && pnpm test:workers && pnpm build-- all green.Issue #1 (native boolean kind) had not yet merged to
mainwhen this branch was cut, soComputedValueSchema/ExpressionNodeSchemadon't include a boolean member here -- this branch is based on currentmainas-is. Worth checking merge order against #1 before merging either.