Skip to content

chore(tesseract): rename view filters to default_filters#10930

Open
waralexrom wants to merge 1 commit into
masterfrom
tesseract-rename-view-filters
Open

chore(tesseract): rename view filters to default_filters#10930
waralexrom wants to merge 1 commit into
masterfrom
tesseract-rename-view-filters

Conversation

@waralexrom
Copy link
Copy Markdown
Member

Summary

Renames the view-level default-value filter key introduced in #10892 from filters to default_filters (camelCase defaultFilters in JS) so the data-model surface reflects that these are default filters, distinct from the per-query filters option.

Renames the view-level default-value filter key introduced in #10892
from `filters` to `default_filters` (camelCase `defaultFilters` in JS)
across the TS schema-compiler, YAML/JS transpilers, Rust bridge,
planner, fixtures, and tests.
@waralexrom waralexrom requested review from a team as code owners May 21, 2026 16:33
@github-actions github-actions Bot added rust Pull requests that update Rust code javascript Pull requests that update Javascript code labels May 21, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 21, 2026

Claude finished @waralexrom's task in 2m 17s —— View job


Code Review: PR #10930

  • Gather context and understand the changes
  • Review the diff for completeness and consistency
  • Check for missed references to old naming
  • Review test coverage
  • Post final review

Summary

This PR cleanly renames the view-level filters key (introduced in #10892) to default_filters / defaultFilters across the entire stack — JS/TS schema compiler, Rust bridge traits, Rust SQL planner, and all tests. The rename is well-motivated: filters was ambiguous in context with per-query filters, access policy filters, and measure-level filters.

Review

Completeness: No missed references — I searched for remaining uses of the old filters name scoped to view definitions across all 18 changed files and surrounding code. All occurrences in CubeValidator, CubeEvaluator, CubeSymbols, YamlCompiler, CubePropContextTranspiler, the Rust CubeDefinition trait, query planner, bridge tests, and YAML test fixtures have been updated. The remaining filters references in the codebase are all unrelated (access policy filters, measure-level filters, query filters, template engine context).

Regex patterns updated correctly — The transpiler patterns in CubePropContextTranspiler.ts:40-42 correctly accept both defaultFilters (JS camelCase) and default_filters (YAML snake_case):

/^(defaultFilters|default_filters)\.[0-9]+\.member$/
/^(defaultFilters|default_filters)\.[0-9]+\.values$/
/^(defaultFilters|default_filters)\.[0-9]+\.unless$/

This is actually more specific than the old ^filters\. prefix, which is a subtle improvement — it eliminates any theoretical false matches with access policy or measure-level filter paths.

YAML-to-JS camelization handledcamelizeCube() in utils.ts converts all top-level snake_case keys to camelCase, so default_filters in YAML files becomes defaultFilters before the YamlCompiler.ts regex checks (/^defaultFilters\.\d+\.member$/ on line 196) — this is consistent.

Rust bridge is consistent — The CubeDefinition trait (cube_definition.rs:44) uses fn default_filters(...) with #[nbridge(field, optional, vec)], which the nbridge macro maps to defaultFilters on the JS side. The mock implementation correctly generates both default_filters() and has_default_filters() methods.

Validator schema renamedViewFilterSchemaViewDefaultFilterSchema in CubeValidator.ts:1133 keeps the internal schema name aligned with the key name.

Test coverage is thorough — The PR updates:

  • Unit validator tests (7 test cases covering valid/invalid operator, missing member, non-view rejection)
  • Schema unit tests (member resolution, unless behavior, error on non-included member)
  • Integration test (view-default-value-filters.test.ts — 4 YAML view definitions)
  • Rust mock tests (empty and populated filter scenarios)
  • Rust YAML test fixtures
  • Bridge coverage test expectations

No backward compatibility concerns — The filters key on views was introduced in the parent PR (#10892) and hasn't been released yet, so renaming it now is the right time before it becomes a public API surface.

Verdict

Looks good to me. Clean, mechanical rename with no logic changes. All references are updated consistently across JS and Rust, the regex patterns are correct, and test coverage is comprehensive.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.93%. Comparing base (9268376) to head (e930f47).

Files with missing lines Patch % Lines
...ubejs-schema-compiler/src/compiler/YamlCompiler.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10930   +/-   ##
=======================================
  Coverage   78.93%   78.93%           
=======================================
  Files         470      470           
  Lines       92868    92868           
  Branches     3450     3450           
=======================================
+ Hits        73307    73309    +2     
+ Misses      19056    19054    -2     
  Partials      505      505           
Flag Coverage Δ
cube-backend 58.42% <60.00%> (-0.01%) ⬇️
cubesql 83.53% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update Javascript code rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant