Skip to content

test(predicates): extend the null-guard sweep to the arguments of stdlib calls - #1366

Merged
os-steve merged 2 commits into
mainfrom
claude/issue-1115-predicate-totality-stdlib-args
Aug 27, 2026
Merged

test(predicates): extend the null-guard sweep to the arguments of stdlib calls#1366
os-steve merged 2 commits into
mainfrom
claude/issue-1115-predicate-totality-stdlib-args

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes #1115

The repo-wide predicate guard, test/object-validation-predicates.test.ts, greps the operands of ordering comparisons for a != null guard. A predicate whose only null-sensitive site is a function argument has no ordering operator at all, so it passed vacuously — green, having checked nothing. This adds the missing half.

The third abort route, measured in PR #1110:

daysBetween(null, …) → BigInt(NaN) → THROWS inside the stdlib function
                     → engine reports `predicate failed to evaluate`
                     → an ordinary save is REJECTED (17.0.0-rc.2, objectstack#4649)

has() does not cover it: a key that is present and null clears has(record.f) and detonates the call anyway. That is why this is the != null family of guard, not the has() family.

⭐ Proven RED before it was accepted — the non-negotiable

Instrument named by the card: take period_end_matches_calendar_period on crm_forecast, strip its != null guards, and the new rule must go red on it. Full log: ablate.sh, run under the shared verify lock.

Mutation leg — proven to have reached disk before anything was measured:

HEAD blob      : 46e8dbcd1f1983bb25a265d6f3b68f0a54b2cd7f
target line    : 428
guards on that line BEFORE : 3
guards on that line AFTER  : 0
on-disk hash now           : 61f36941171b33e0656efc86909d5698ab986dcb
MUTATION CONFIRMED ON DISK (guard count 3 -> 0, git diff non-empty)

RED, and red for the right reason — the rule names the predicate and both fields:

Tests  1 failed | 77 passed (78)

FAIL  every stdlib call argument is null-guarded
      > hands no field to a stdlib call without a != null guard

- []
+ [ { "id": "crm_forecast.period_end_matches_calendar_period",
+     "unguarded": [ "period_end", "period_start" ] } ]

Restore leg — proven restored, not assumed:

on-disk hash after restore: 46e8dbcd1f1983bb25a265d6f3b68f0a54b2cd7f
HEAD blob                 : 46e8dbcd1f1983bb25a265d6f3b68f0a54b2cd7f
RESTORE CONFIRMED ON DISK (hash == HEAD blob, git diff HEAD empty)

The mutation was applied to a committed implementation, so the restore leg had a real reference point; the script carries trap restore EXIT INT TERM with an absolute REPO_ROOT, and an empty hash is treated as failure rather than "nothing to compare".

The measurement that justifies the whole card

Under that same mutation, 77 tests still passed — including every engine-driven totality test in the file. The new sweep was the only thing in the repo that caught the stripped predicate.

Why the engine half stays green, measured rather than assumed: with every field null, record.period == "month" is false and record.period == "quarter" is false, so && short-circuits and daysBetween(…) is never reached. Only the mixed shape — period and period_start set, period_end null — reaches the call, and no sweep in this file constructs that shape. That is the card's claim, now a measurement.

⚠️ Vacuity guard on the new rule itself

A matcher that finds zero call sites is green forever and worthless — and from the outside it is indistinguishable from a clean tree, which is the exact defect this card records. So the new sweep asserts its own reach first:

  • it locates a non-zero set of policed call sites across all 37 authored predicates;
  • on the anchor predicate it names daysBetween, addDays and addMonths by function;
  • the paren-balanced scan reaches both reads — ['period_end', 'period_start']. An innermost-only [^()]* match would see addMonths(…) and miss record.period_end, the argument that actually detonates;
  • both directions of the matcher are proved against fixtures, so its ability to go red does not depend on the tree ever being wrong.

How the rule decides

Deny by default. Every field read inside a function call must carry record.f != null in the same expression, unless the function is on a short list measured not to abort on null: has, isBlank, coalesce — the measurement is the table already in this file's HOUSE RULE block. The direction is the point: the next stdlib function someone reaches for is guarded before anyone measures it. isBlank has to be exempt or the house rule's own "f holds no value" shape, (!has(record.f) || isBlank(record.f)), becomes an offender in 13 shipped predicates.

String-literal contents are blanked (length preserved, offsets intact) so a regex literal like "^[0-9]{4}-(01|04|07|10)-01" cannot confuse the paren balance and a record. spelled inside a literal is not a phantom field read.

No exemption list was needed: the tree is clean as it ships.

Scope

  • In scope, done: the static half only.
  • ⛔ Deliberately NOT done: the engine-driven half (mixed null shapes, one field null at a time). The card calls it "strictly stronger and strictly more expensive", and it needs its own pricing. No fork was hit — the static half holds this class on its own, as the ablation above shows.
  • Nothing else was touched: no predicate, no unrelated gate, no upstream behaviour.

One thing for the reviewer to judge

src/objects/forecast.object.ts carries a comment saying this predicate "has no ordering operator, so object-validation-predicates.test.ts's unguarded-comparison sweep does not cover it — the pin lives in this card's own test file instead." That sentence stays literally true (the unguarded-comparison sweep still does not cover it; this is a different sweep), but its implication is now weaker, since the class is covered repo-wide. I left it alone rather than widen a test-only diff into src/ on my own judgement. Say the word and it is a two-line follow-up.

Verification

pnpm verify green — all eight stages ran (validate, typecheck, lint, lint:i18n-gate, hygiene, hygiene:tokens, build, test): 145 test files, 3033 passed, 1 skipped, verdict command-exit 0 from the shared verify lock.

Re-run on the shipping sha 6dd49bd with a clean tree: ✓ source hygiene clean, ✓ source token ratchet clean, and the sweep file 78 passed (78) — up from 75, the three new tests.

Changeset frontmatter is empty, and that was verified rather than assumed: git diff origin/main --stat touches test/ and .changeset/ only, so this PR ships nothing to HotCRM users.


Generated by Claude Code

claude added 2 commits August 27, 2026 12:50
The repo-wide predicate sweep greps the operands of ORDERING comparisons.
A predicate whose only null-sensitive site is a function ARGUMENT has no
ordering operator at all, so it passed vacuously — green, having checked
nothing. daysBetween(null, ...) reaches BigInt(NaN) and throws inside the
function, which the engine reports as `predicate failed to evaluate` and,
from 17.0.0-rc.2, turns into a REJECTED ordinary save.

Adds a balanced-paren scan of every function call in every authored
predicate, deny-by-default against a measured null-tolerant allowlist,
with a vacuity guard that asserts the matcher reaches a non-zero set of
call sites and both fixture directions of the matcher itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WMzCeNC4SZcPNBpE2zCVCg
Test-only diff, so the frontmatter is deliberately empty — the sanctioned
"releases nothing" declaration, verified against the diff rather than
assumed: git diff origin/main touches test/ and .changeset/ only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WMzCeNC4SZcPNBpE2zCVCg
@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Aug 27, 2026 1:01pm

Request Review

@github-actions github-actions Bot added the ci/cd CI plumbing and the verification pipeline label Aug 27, 2026
@os-steve
os-steve marked this pull request as ready for review August 27, 2026 13:05
@os-steve
os-steve added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 69bcea7 Aug 27, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd CI plumbing and the verification pipeline

Projects

None yet

2 participants