Skip to content

test(core): make the filter parity suite able to catch a wrong created_at bound - #1042

Merged
cameri merged 1 commit into
rewrite/mainfrom
factory/021-created-at-bound
Nov 3, 2026
Merged

cameri merged 1 commit into
rewrite/mainfrom
factory/021-created-at-bound

Conversation

@chappie-daemon

Copy link
Copy Markdown
Collaborator

Closes #981 — see the closing note at the end: the keyword is inert on this base.

The filter parity suite could not see a wrong created_at bound. Measured before this change: moving the bound from CREATED_AT_LIMIT (10¹⁰) to 10⁸ left the suite at 54 passed / 0 failed, because the nearest invalid row was 1_672_295_751_103 — 167× the real limit — and every other row sat far below 10⁸.

The sharper half, which the issue did not measure: a filter-only mutation (filter.rs:487 → CREATED_AT_LIMIT / 100, decoder untouched) left the entire workspace at 942 passed / 0 failed. So a wrong filter bound was caught by nothing anywhere, not merely missed by this suite.

What changed

Four rows in a new section of crates/nostream-core/tests/filter_schema_parity.rs, split by direction:

row asserts fails under
since / until = 9_999_999_999 accepted a bound that is too low (10⁸, /100)
since / until = 10_000_000_000 refused a bound that is too high (10¹²)

Two files, +54/−0: the tests (+46) and an empty-fronted changeset (+8). No production line moved — git diff --stat 0fa4150..8b131dd -- crates/nostream-core/src/ is empty.

The oracle, run as inversions rather than argued

Each in its own git archive copy with its own CARGO_TARGET_DIR, mutation confirmed by grep before running:

run mutation at base at this commit
A validation.rs:20 → 10⁸ 54/0, exit 0 56/2, exit 101 — the two acceptance rows
B filter.rs:487 → CREATED_AT_LIMIT / 100 54/0, exit 0 56/2, exit 101 — the two acceptance rows
C validation.rs:20 → 10¹² — 56/2, exit 101 — the two refusal rows

A and B never list the refusal rows; C never lists the acceptance rows. That asymmetry is the point of splitting by direction: a row written to the wrong side would show up as a green inversion rather than a silent one.

cargo fmt --all --check                                    exit 0
cargo clippy --workspace --all-targets --all-features -D warnings   exit 0
cargo clippy --workspace --lib --bins (strict five lints)  exit 0
cargo test --workspace --all-features                      exit 0   946 passed / 0 failed (942 before)
cargo build --workspace --all-features --release           exit 0

Parity suite 54 → 58.

Where the numbers come from

10¹⁰ is the capture's own predicate — Math.log10(value) < 10 in src/schemas/base-schema.ts — evaluated rather than read: node accepts 9999999999 and refuses 10000000000. The capture's table has no row near the bound either, which is why the suite agreed with itself.

The literals cannot be shared with the implementation: CREATED_AT_LIMIT is pub(crate) and a test is a separate crate. That is not a limitation to work around — an expectation read from the constant would have stayed green under all three inversions above.

Left uncovered, stated

  • The decoder's exact accepted top (9_999_999_999) is still untested on the event surface. A decoder-only ceiling between 1_700_000_000 and 10_000_000_000 would be invisible; the shared constant is now pinned by these four rows, so the two cannot drift apart silently.
  • pnpm exec changeset status was not run (no node_modules in the worktree); CI's Changeset Required is its first real run.

Note on the keyword

Closes #981 does nothing here: GitHub closes by keyword only when the pull request targets the default branch, and this targets rewrite/main. Whoever merges closes the issue by hand and reads the state back.

@changeset-bot

changeset-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8b131dd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f73b4ace-e6af-4451-8cd5-8476da645124

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@greptile-apps

greptile-apps Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

Greptile CLI A Greptile CLI review exists for this PR, but a new review ran because auto-approve needs this PR's own review.

[Low impact] This PR appears safe to merge.

Summary

Adds four tests that pin the timestamp ceiling for since and until.

  • The filter parity suite checks both edges of the created_at limit.

Reviews (1) · Last reviewed commit: "test(core): pin the filter's created_at ..." · Reviewed by Greptile

@cameri
cameri merged commit 15c4261 into rewrite/main Nov 3, 2026
16 checks passed
@cameri
cameri deleted the factory/021-created-at-bound branch November 3, 2026 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants