Skip to content

fix(engine): drop never-gated hidden array items instead of null holes - #1217

Merged
guitavano merged 2 commits into
mainfrom
guitavano/fix-never-matcher-null-array
Jul 23, 2026
Merged

fix(engine): drop never-gated hidden array items instead of null holes#1217
guitavano merged 2 commits into
mainfrom
guitavano/fix-never-matcher-null-array

Conversation

@guitavano

@guitavano guitavano commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Same fix as the blocks runtime, ported to the deco engine.

Hiding an array item wraps it in a multivariate flag gated by a never matcher. blocks/flag.ts returns match?.valueundefined when no variant matched. The engine's resolvePropsWithHints wrote that undefined back into the resolved array (mutableProps[index] = undefined), leaving a hole that serializes to null in JSON and renders as an empty card (blank benefit rows, empty banners, etc.).

Fix (engine/core/resolver.ts)

Compact undefined out of resolved arrays before returning. A resolver returning null is a legitimate value and is kept — only the undefined "not present" sentinel is filtered.

Testing

Reproduced empirically first (array with a resolver returning undefined → serialized to [...,null,null]), then confirmed the fix drops the holes.

Added a regression step to engine/core/mod.test.ts (drops hidden array items (resolver -> undefined) but keeps null). Full resolve suite: 8/8 steps pass. deno fmt/deno lint clean.

🤖 Generated with Claude Code


Summary by cubic

Drop hidden array items in the engine instead of leaving null holes. Prevents empty cards from undefined entries and skips extra work when arrays have no hidden items.

  • Bug Fixes
    • Compact undefined from resolved arrays; only reallocate if the array contains undefined. Preserve null.
    • Broadened tests: all-hidden arrays resolve to [], nested arrays drop hidden items, and null is kept.

Written for commit a6f2afd. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Fixed array property resolution to drop elements that resolve to undefined, avoiding empty slots in results.
    • Preserved explicitly resolved null values so they remain intact in array outputs.
  • Tests

    • Added additional test coverage for hidden/omitted items within arrays, including nested arrays and the case where all elements are hidden.

Hiding an array item wraps it in a multivariate flag gated by a `never`
matcher; flag.ts returns `undefined` when no variant matched. The engine
left that `undefined` as a hole in the resolved array, which serializes
to `null` in JSON and renders as an empty card (blank benefit rows,
etc.).

Compact `undefined` out of resolved arrays in resolvePropsWithHints. A
resolver returning `null` is a legitimate value and is kept — only the
`undefined` "not present" sentinel is filtered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 1.202.2 update
  • 🎉 for Minor 1.203.0 update
  • 🚀 for Major 2.0.0 update

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Array resolution now removes undefined results from untyped arrays while preserving null, including nested arrays. Tests cover mixed, all-hidden, and nested array results.

Changes

Array resolution

Layer / File(s) Summary
Filter undefined array results
engine/core/resolver.ts, engine/core/mod.test.ts
Untyped array results filter out undefined elements while preserving null; tests cover mixed, all-hidden, and nested arrays.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: compacting hidden array items instead of leaving null-like holes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch guitavano/fix-never-matcher-null-array

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

Address review feedback:
- Only reallocate when the array actually contains `undefined` (scan with
  `includes` first) so the common zero-undefined case skips the extra
  allocation on the hot resolve path.
- Reframe the comment around the generic invariant (object props already
  drop `undefined` under JSON.stringify; array `undefined` serializes to a
  `null` hole — compaction makes arrays consistent), citing the never-gated
  flag as the motivating case rather than the definition.
- Add tests: all-items-hidden -> [], and nested-array multi-depth (with a
  first-element drop to pin ordering).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@guitavano
guitavano merged commit 427a37c into main Jul 23, 2026
4 checks passed
@guitavano
guitavano deleted the guitavano/fix-never-matcher-null-array branch July 23, 2026 01:19
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