fix(tesseract): Resolve join for hint-less member expressions on views

[MazterQyou]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Description of Changes Made

This PR fixes the invalid type: unit value, expected struct JoinDefinitionStatic error for queries whose member expressions on a view carry no join hints of their own, such as count(*) alongside count(distinct <view dimension>). Related tests are included.

[claude]

Claude finished @MazterQyou's task in 4m 5s —— View job

---

Review: approach is sound — 2 medium, 3 low (5 inline comments)

Full review

• Gather context (diff, multi_fact_join_groups.rs, join_hints.rs, query_properties.rs, fixtures, seed data)
• Understand the request (comprehensive code review)
• Review code quality / correctness
• Review test coverage
• Check security & performance angles
• Post inline comments and summary

Verdict

The diagnosis is right and the fix is in the right place. The old Ok(JoinHints::new()) branch for views produced a hint set that nothing downstream could resolve, and the bridge then failed with invalid type: unit value, expected struct JoinDefinitionStatic — an error that told the user nothing. Borrowing the query's other hints for a hint-less view member expression, and hard-erroring with a CubeError::user when there is nothing to borrow, is a clear improvement on both counts.

I also checked the pooled_hints preservation in for_measures, which reads odd at first: it is in fact load-bearing. query_properties.rs:590 calls compute_join_multi_fact_groups_with_measures(&[m]) for a single measure and then .expect()s a join group — without preserving the whole-query pooled hints, a hint-less view ME on its own would fall through to base_hints and could panic there. Worth keeping; the doc comment covers it.

Verified the new snapshot against the seed data: integration_multi_fact_tables.sql has 8 orders across 2 distinct statuses, so 2 | 8 is correct. The #[test] (non-tokio) error test is consistent with the existing sync error tests in the same file (test_subquery_join_unknown_join_type, test_subquery_join_no_cube_reference_in_on).

Findings

#	Sev	Where	Issue
1	Medium	multi_fact_join_groups.rs:126-129	pooled_hints re-appends base_hints once per measure (each mh.hints already contains it), and JoinHints::push dedups only adjacent entries → repeated items, distinct join_tree_cache key, redundant bridge resolution
2	Medium	multi_fact_join_groups.rs:285	For a genuinely multi-fact query, pooled hints are the union across all facts, so the hint-less view ME forms its own join group over a fan-out tree — COUNT(*) then counts a cross product. Untested; customer_overview in the fixture is exactly this shape
3	Low	multi_fact_join_groups.rs:232-240	Error text says "no other member to resolve the join from", but multi-stage measures are filtered out before pooling, so it can fire when other members do exist
4	Low	multi_fact_join_groups.rs:97-101	for_measures constructs then overwrites pooled_hints; passing it into from_base_hints would make the invariant structural
5	Low	member_expressions.rs:570	Stray trailing blank line at EOF

Security / performance

No security surface here — no user input reaches SQL text through this path; the change only reorders which cube hints seed join resolution. Performance impact is confined to finding 1 (avoidable join_tree_cache misses from non-canonical hint lists); the pooled_hints field itself is one extra JoinHints clone per MeasuresJoinHints, which is negligible.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed. The doc comments added to MeasuresJoinHints, for_measures and fallback_hints_for_measure are genuinely good — they explain the why (legacy-parity pooling) rather than restating the code.

What I could not verify

I did not compile or run the tests: this is a fresh checkout with no Cargo build cache for the cubesqlplanner crate, and a cold build of the Rust workspace exceeds the time available in this job. All findings above are from static reading of the code plus the fixtures; finding 2 in particular is reasoned from the join-graph shape in integration_views.yaml and should be confirmed by actually running the suggested customer_overview case. The positive-path test also only asserts against Postgres when one is available (try_execute_pg_from_options returns Option), so the snapshot may be skipped in some environments.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.49%. Comparing base (a0723ac) to head (4b2f485).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11501      +/-   ##
==========================================
+ Coverage   79.47%   79.49%   +0.01%     
==========================================
  Files         480      480              
  Lines       98861    98928      +67     
  Branches     3636     3636              
==========================================
+ Hits        78568    78638      +70     
+ Misses      19773    19771       -2     
+ Partials      520      519       -1     

Flag	Coverage Δ
cube-backend	59.28% <ø> (+0.04%)	⬆️
cubesql	83.97% <ø> (+0.01%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

[claude]

Claude finished @MazterQyou's task in 5m 2s —— View job

---

Re-review: all 5 prior findings addressed — 1 medium, 3 low (4 new inline comments)

Full review

• Gather context (new diff vs 259188d, planner, fixtures, snapshots, seed data)
• Check which of the 5 prior findings are addressed
• Review the new join_map fallback + mock fixture change
• Verify snapshot numbers against seed data
• Resolve stale threads, post new findings

Prior findings — all addressed, threads resolved

#	Prior finding	Resolution
1	pooled_hints re-appended base_hints per measure	Now pools only the incremental own_hints, with a comment on why canonicality matters
2	Multi-fact pooled hints → own fan-out group, untested	test_expr_measure_count_star_no_hints_on_multi_fact_view locks it in, and the intent is documented
3	Error text claimed "no other member" while multi-stage were filtered pre-pooling	Multi-stage measures now contribute to the pool (continue after pooling), and the message is rewritten
4	for_measures constructed then overwrote pooled_hints	Threaded through as inherited_pooled_hints: Option<JoinHints>
5	Trailing blank line at EOF	Gone

Snapshot arithmetic — checked against integration_multi_fact_tables.sql

• no_hints_on_view: 2 | 8 — 8 orders, 2 distinct statuses. ✅
• no_hints_on_multi_fact_view: 8 | 5 | 13 — the fan-out over customers⋈orders⋈returns is c1 4×1=4, c2 3×2=6, c3 0→1×2=2, c4 1×0→1=1 = 13. ✅ The two real measures stay in their own narrow groups (8 and 5), so the multi-fact grouping is doing the right thing.
• only_member_on_view_with_join_map: 4 — customers row count via the join-map root. ✅

New findings

#	Sev	Where	Issue
1	Medium	multi_fact_join_groups.rs:307-316	The join-map last resort only fires for multi-hop views (joinMap is filtered to len > 1), so COUNT(*) alone still hard-errors on a single-cube view like orders_view — the commonest view shape, and precisely the BI-profiling case the PR body cites
2	Low	multi_fact_join_groups.rs:309-313	Root taken from the first joinMap path → depends on view cube ordering; and COUNT(*) on customer_overview means 4 alone vs 13 alongside other measures. Parity with legacy, but should be documented
3	Low	mock_schema.rs:648-661	Populating join_map changes hint collection for every existing view fixture (Single(x) → Vector([x]), a distinct join_tree_cache key). Only 3 assertions needed updating, which is plausible — please confirm the full suite is green
4	Low	multi_fact_join_groups.rs:112-120	JoinHints::push dedups only adjacent Singles, so identical Vectors still repeat in pooled_hints — pre-existing, but pooled_hints is now a cache key of its own

Finding 1 is the only one I'd want a decision on before merge, and "out of scope, follow-up filed" is a perfectly good answer — the current state is strictly better than the invalid type: unit value, expected struct JoinDefinitionStatic failure either way. The rest are comment/robustness polish.

Code quality

The from_base_hints rewrite is a genuine improvement over the previous filter-then-map: one pass, and the "multi-stage measures get no entry but do count towards the pool" rule is now stated where it happens rather than implied by a filtered_measures vector two blocks up. The inherited flag reads slightly better than a is_some() re-check would, though match inherited_pooled_hints { Some(p) => …, None => … } would make the two modes structurally exclusive rather than coupled through a bool.

The error message is specific and actionable. It interpolates cube_name twice (as the referenced cube and as the join-map owner) which reads a little repetitively for the common case where they're the same string, but it is correct.

Security / performance

No security surface: nothing user-controlled reaches SQL text through this path, and the change only reorders which cube hints seed join resolution. Performance is a wash — collect_join_hints is now also called for multi-stage measures (one extra traversal each, previously skipped), offset by no longer cloning base_hints into the pool per measure. pooled_hints adds one JoinHints clone per MeasuresJoinHints.

Testing

Four new tests covering the three fallback branches plus the terminal error, which is the right decomposition. Two gaps: the positive assertions only fire when a Postgres is reachable (try_execute_pg_from_options returns Option), so in environments without one those three tests degrade to "the planner didn't panic"; and there's no case for a hint-less view ME alongside only multi-stage measures, which is the path the reworked pooling in from_base_hints specifically enables.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed. The doc comments carry real weight here — they explain the legacy-parity reasoning rather than restating the code, and the per-test comments state the expected numbers and why, which is what makes the 13 in the multi-fact snapshot reviewable at all.

What I could not verify

I could not build or run the tests — cargo check on the cubesqlplanner crate isn't permitted in this job's sandbox, and this is a cold checkout with no build cache. Every finding above is from static reading of the diff, the fixtures, the seed SQL and the JS schema compiler; the snapshot arithmetic is hand-computed. Finding 3 in particular is a request for confirmation, not a claimed regression.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[claude]

Claude finished @MazterQyou's task in 4m 36s —— View job

---

Re-review: 3 of 4 prior findings addressed — 0 medium, 2 low (2 new inline comments)

Full review

• Gather context (current diff vs commit parent, planner, fixtures, snapshots, seed data)
• Check which of the 4 prior unresolved findings are addressed
• Review the new ambiguous-root error + two_roots_view fixture + push dedup change
• Check blast radius of the shared fixture addition
• Resolve stale threads, post new findings

Prior findings

#	Prior finding	Resolution
1	Join-map last resort only rescues multi-hop views; single-cube views still error	Documented as an explicit known limitation on test_expr_measure_count_star_only_member_on_view, with the two lift options and why they're out of scope. Thread resolved — the ask was "say so if intentional", and it now does.
2	Root taken from the first join-map path (order-dependent); query-dependent meaning undocumented	Multiple distinct roots now raise a CubeError::user instead of picking arbitrarily, covered by two_roots_view + ..._with_ambiguous_join_map; the query-dependent meaning is spelled out on fallback_hints_for_measure. Thread resolved.
3	join_map in the mock changes hint collection for every view fixture — asked for confirmation the suite is green	Left open. The fidelity divergence is now documented in the mock, but I still can't run the crate (see below), so the confirmation ask stands.
4	Vector repeats not deduped in push	push now drops any verbatim repeat of the tail, with test_push_skips_repeated_vector covering both the drop and the "adjacent only" boundary. Thread resolved.

What I re-verified

• The new ambiguous-root error is narrow: fallback_hints_for_measure only reaches it when pooled_hints is empty, i.e. the hint-less view ME is the sole contributor of hints in the whole query. A two-root view queried alongside any real member still resolves through pooling, so this can't reject anything that used to work — the prior behaviour for that shape was the invalid type: unit value bridge failure anyway.
• customer_overview's mock join map is [[customers, orders], [customers, returns]] → one unique root customers → the 4 snapshot. two_roots_view's is [[orders, customers], [returns, customers]] → two roots → error. Both consistent with the > 1 path filter.
• Adding two_roots_view to the shared integration_views.yaml is safe: no test in tests/integration/views.rs enumerates views or asserts a count, and its two includes (customers.city, customers.name) exist and don't collide.
• for_measures is only ever called with subsets of the query's own measures (query_properties.rs:590, multiplied_measures_query_planner.rs:71,117), so inheriting pooled hints rather than recomputing them is sound — the subset's hints are already in the parent pool.

New findings

#	Sev	Where	Issue
1	Low	join_hints.rs:9-10	The new "canonical, because it's a cache key" claim isn't enforced: from_items bypasses push, and it's the exit path of collect_join_hints; prefix subsumption ([V[a], V[a,b]] vs [V[a,b]]) still yields distinct keys for equivalent hints
2	Low	multi_fact_join_groups.rs:117-129	The "multi-stage measures pool but get no entry" branch — the one that makes the reworked error message honest — has no test; plus collect_join_hints runs a wasted full traversal per multi-stage measure on the inherited path

Neither blocks. Finding 1 is about a doc claim over-promising an invariant one constructor can break; finding 2 is a coverage gap plus a mechanical reorder.

Code quality

The Option<JoinHints> threading reads well, and moving the multi-stage check into the loop after pooling puts the rule where it happens. The inherited bool is the one spot I'd still nudge — the two modes are mutually exclusive but coupled through a flag rather than structurally; the suggestion in finding 2 makes it slightly worse in that respect, so a match over the Option up front would be the cleaner version of both.

The ambiguous-root rejection is the right call over picking a root: the alternative is a silent result change on a YAML reordering with no diagnostic. The error text names the actual cause. The doc block on fallback_hints_for_measure has grown long but every paragraph earns its place — the fallback chain, the join-map last resort with its limitation, and the query-dependent-meaning warning are all things a reader would otherwise have to reconstruct.

Testing

Five tests now, covering all three fallback branches plus both terminal errors, which is the right decomposition. Remaining gaps: the multi-stage-only pooling path (finding 2), and the three positive assertions only fire when a Postgres is reachable (try_execute_pg_from_options returns Option) — without one they degrade to "the planner didn't panic". The two error tests are sync #[test] and always assert, consistent with the existing error tests in the file.

Security / performance

No security surface — nothing user-controlled reaches SQL text through this path; the change only decides which cube hints seed join resolution. Performance is roughly neutral: collect_join_hints now also runs for multi-stage measures (finding 2 trims the avoidable half), offset by no longer cloning base_hints into the pool per measure, and the push change removes some cache-key non-canonicality rather than adding any.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed. The comments carry real weight in this round — the mock's note on why join_map filters single-element paths (and what that does to Single vs Vector hints) is the kind of thing that would otherwise be rediscovered by whoever next debugs a cache-key mismatch.

What I could not verify

I could not build or run anything: there is no Cargo target dir in this checkout and a cold build of the Rust workspace exceeds this job's budget; git fetch origin master is also not permitted here, so the diff was taken against the commit's parent (259188d, which is the current master tip in this checkout) rather than a fetched origin/master. The previously-reviewed head (55a179d) isn't present locally either, so the "prior findings" table is judged from the current file contents, not a commit-to-commit diff. Snapshot arithmetic is hand-computed against integration_multi_fact_tables.sql. Prior finding 3 remains a request for confirmation, not a claimed regression.

· branch tesseract/resolve-join-hint-views ·

[claude]

Claude finished @MazterQyou's task in 4m 26s —— View job

---

Re-review: both prior code findings addressed — 1 medium, 1 low (2 new inline comments), 1 prior thread still awaiting confirmation

Full review

• Gather context (diff vs origin/master, planner, fixtures, snapshots, seed data)
• Check which prior unresolved findings are addressed
• Review the new amount_share multi-stage fixture + test, and the join_hints doc rewrite
• Verify snapshot arithmetic
• Resolve stale threads, post new findings

Prior findings

#	Prior finding	Resolution
1	join_hints.rs doc promised a canonicality invariant from_items can break; prefix subsumption unhandled	Doc rewritten to say exactly that — "a local rule, not a normal form", from_items stores as-is, prefixes aren't collapsed, and unequal hints aren't proof of different trees. Thread resolved.
2	Multi-stage pooling branch untested + wasted collect_join_hints traversal on the inherited path	orders.total_amount_by_status / orders.amount_share added to the fixture and test_expr_measure_count_star_no_hints_beside_multi_stage_measure covers the branch; the is_multi_stage && inherited check now short-circuits before collect_join_hints. Thread resolved.
3	join_map in the mock changes hint collection for every view fixture — asked for confirmation the suite is green	Still open. The divergence is documented in the mock now, but I still can't build the crate here, so the confirmation ask stands.

What I verified

Snapshot arithmetic, hand-checked against integration_multi_fact_tables.sql:

• no_hints_on_view → 2 | 8 ✅ (8 orders, 2 distinct statuses)
• no_hints_on_multi_fact_view → 8 | 5 | 13 ✅ (fan-out: 4+6+2+1)
• only_member_on_view_with_join_map → 4 ✅ (customers row count via the join-map root)
• no_hints_beside_multi_stage_measure → 1.1956… | 8 and 6.1111… | 8. The two rows are add_group_by: orders.status leaking granularity into a dimension-less query, which is pre-existing behaviour — multi_stage__add_group_by__add_group_by_no_granularity.snap shows the same shape with 15 rows. The ratios are self-consistent: 1/1.1956 + 1/6.1111 = 1.0000, i.e. the two status subtotals sum to the whole, so amount_share = total / total_by_status is being computed correctly. ✅

Also checked: the test is genuinely load-bearing for the pooling branch — base_hints comes only from initial_hints + dimensions + filters (multi_fact_join_groups.rs:47-59), never from measures, so with no dimensions the pool is the only thing that gives COUNT(*) a cube. And two_roots_view is safe to add to the shared integration_views.yaml: nothing in tests/integration/views.rs enumerates views or asserts member counts, and the new orders measures aren't referenced by existing queries.

New findings

#	Sev	Where	Issue
1	Medium	multi_fact_join_groups.rs:318-320	Pooled hints are borrowed with no check that they contain any cube of the view. A COUNT(*) ME on orders_view alongside customer_overview.returns_count resolves against the customers ⋈ returns tree — orders nowhere in it. Silently wrong number where today there's a loud bridge error
2	Low	multi_fact_join_groups.rs:325-339	The ambiguous-root check is literal-first-element equality, not reachability, so [[customers, orders], [orders, line_items]] is rejected despite having one real root; and the error can't name the conflicting cubes because the iterator is consumed to detect them

Finding 1 is the only one I'd want a decision on. "Out of scope, single-view assumption documented" is a fine answer — but the doc block's "the members it borrowed from" currently implies a same-view constraint the code doesn't enforce, so either the check or the wording should move.

Code quality

The is_multi_stage && inherited short-circuit reads better than the version it replaces — the three continue/pool/entry cases are now visibly distinct, and each carries the one line of reasoning a reader needs. The join_hints.rs doc rewrite is the standout: turning "adjacent redundant entries are silently dropped" into an explicit statement of what the rule doesn't guarantee, plus the consequence for cache-key equality, is more useful than the invariant it used to claim.

The new fixture comment on amount_share ("not multi-stage itself, but depends on a multi-stage measure") is exactly the note that makes the test's purpose legible — without it the measure looks like an arbitrary ratio.

The error messages are specific and name the member. The first still interpolates cube_name twice, which reads repetitively when the referenced cube and the join-map owner are the same string, but it's correct.

Testing

Six tests now, covering all three fallback branches plus both terminal errors plus the multi-stage pooling path. The remaining gap is unchanged and structural: the four positive assertions only fire when a Postgres is reachable (try_execute_pg_from_options returns Option), so without one they degrade to "the planner didn't panic". The two error tests are sync #[test] and always assert.

No test covers finding 1's shape (hint-less view ME + a measure from a cube outside that view), which is what let it through.

Security / performance

No security surface — nothing user-controlled reaches SQL text through this path; the change only decides which cube hints seed join resolution. Performance is now slightly better than before the reorder: the inherited path skips a full symbol traversal per multi-stage measure, and push dropping verbatim repeats removes some join_tree_cache key non-canonicality. pooled_hints costs one JoinHints clone per MeasuresJoinHints.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed.

What I could not verify

I could not build or run anything — cargo check is not permitted in this job's sandbox and this is a cold checkout with no target dir. Every finding is from static reading of the diff, the fixtures, the seed SQL and the collector; the snapshot arithmetic is hand-computed. Finding 1's failure scenario is reasoned from the join-graph shape in integration_views.yaml and the control flow in fallback_hints_for_measure, not observed. Prior finding 3 remains a request for confirmation, not a claimed regression.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[claude]

Claude finished @MazterQyou's task in 4m 52s —— View job

---

Re-review: prior medium finding fixed structurally — 0 medium, 3 low (3 inline comments), 1 prior thread still awaiting confirmation

Full review

• Gather context (diff vs origin/master, planner, fixtures, snapshots, seed data)
• Check which prior unresolved findings are addressed
• Review the new hints_by_cube grouping, reachability-based root rule, nested_root_view fixture
• Re-verify snapshot arithmetic and blast radius
• Resolve stale threads, post new findings

Prior findings

#	Prior finding	Resolution
1	Pooled hints borrowed unconditionally — a hint-less view ME could resolve against a join tree its view has no part in	Fixed at the data-structure level rather than with a guard: hints are now grouped by the owning cube (hints_by_cube), and a view ME may only borrow the entry keyed by its own view. test_expr_measure_count_star_no_hints_beside_other_view_measure locks in the exact orders_view + customer_overview.returns_count shape I raised. Thread resolved.
2	Root rule was literal-first-element equality, not reachability	Now filters heads that any other path walks through, with nested_root_view covering the returns.customers / customers.orders case. The "name the conflicting roots" half is still open — re-raised as new finding 2. Thread resolved.
3	join_map in the mock changes hint collection for every view fixture — asked for confirmation the suite is green	Still open. cargo check is not permitted in this job either, so the confirmation ask stands.

What I verified

The same-view restriction is not a behaviour narrowing: before this PR, a hint-less view ME always got Ok(JoinHints::new()) and died in the bridge, so every path reachable here was previously a hard failure. Nothing that worked can stop working.

Re-checked the snapshots survive the switch from whole-query pooling to per-cube grouping:

• no_hints_on_view → 2 | 8: distinct_status is on orders_view, so hints_by_cube["orders_view"] = [Single(orders)] — same tree as before. ✅
• no_hints_beside_multi_stage_measure: amount_share is on orders_view too, and it's pooled before the multi-stage continue, so the ME still gets orders. ✅
• no_hints_on_multi_fact_view → 8 | 5 | 13: both real measures are on customer_overview, so the view's entry is still the union over both facts and the fan-out count is unchanged (4+6+2+1). ✅
• only_member_on_view_with_join_map → 4: customer_overview join map [[customers, orders], [customers, returns]], reached = {orders, returns}, single unreached head customers. ✅

Root rule against the new fixtures: nested_root_view [[returns, customers], [customers, orders]] → reached = {customers, orders} → sole root returns; two_roots_view [[orders, customers], [returns, customers]] → reached = {customers} → two roots → rejected. Both match their tests.

Also checked for_measures is still only ever handed subsets of the query's own measures (query_properties.rs:340 ← :590 over all_used_measures(), and MultiFactJoinGroups::for_measures), so inheriting hints_by_cube wholesale is sound; and that build_groups' new empty-hints check returns a CubeError rather than an empty group vec, so the .expect("No join groups returned…") at query_properties.rs:592 can't be turned into a panic by it.

New findings

#	Sev	Where	Issue
1	Low	multi_fact_join_groups.rs:54-61	The dimension/filter half of hints_by_cube can never be read — base_hints absorbs the same hints, and a non-empty base_hints means the fallback never fires. Dead work per query, and it makes HintsByCube::Seed read as a source it isn't
2	Low	multi_fact_join_groups.rs:348-370	Ambiguous-root error still can't name the conflicting cubes (roots.next().is_some() consumes the iterator); and a cyclic join map falls into the None arm and reports the wrong cause
3	Low	integration_views.yaml:59-62 + two test comments	Stale "pooled hints" / "the pool" wording now that pooled_hints is gone — greps to nothing, and under-sells that the union is per-view, not per-query

None block. All three are polish.

Code quality

Replacing the pooled_hints field with hints_by_cube is the better fix by some margin — the previous round's problem was that "borrow from the members it came from" was a claim in a doc comment with nothing enforcing it, and now the map simply has no way to hand a view the hints of another view. The HintsByCube::Seed / Inherited enum is clearer than the Option<JoinHints> + inherited bool it replaces: the two modes are structurally exclusive, and the match at the top of from_base_hints names them once.

The three-way continue/collect/entry flow in the measure loop still reads well, and the multi-stage short-circuit on the inherited path is preserved.

The reached set is the right generalisation, and I like that nested_root_view's comment says outright that real view YAML can't produce the shape — it explains why the fixture exists without pretending it's a user-facing case.

Testing

Eight new tests now, covering all three fallback branches, the cross-view rejection, the nested root, and both terminal errors. Two notes:

• test_..._with_nested_join_map asserts only sql.contains("returns"). That does discriminate (a customers-rooted plan wouldn't mention returns), but assert!(!sql.contains("customers")) alongside it would fail more informatively if the rule ever regresses to "first head wins".
• Unchanged and structural: the four positive tests only assert when a Postgres is reachable (try_execute_pg_from_options returns Option); without one they degrade to "the planner didn't panic". The four error tests are sync #[test] and always assert.

One residual, outside this diff

aggregate_subquery_measure (multiplied_measures_query_planner.rs:275) builds its join from collect_join_hints_for_measures(&measures) and passes the result straight to make_join_logical_plan_with_join_hints, with no fallback in between. A hint-less view ME routed there would still hit the raw invalid type: unit value bridge error. I think it's hard to reach — the aggregate-subquery path takes multiplied measures, and a COUNT(*) ME with no member references isn't one — but it's the one remaining place where empty hints reach the bridge unguarded. Worth a follow-up rather than anything in this PR.

Security / performance

No security surface — nothing user-controlled reaches SQL text through this path; the change only decides which cube hints seed join resolution. Performance: one HashMap<String, JoinHints> clone per for_measures instead of the previous single JoinHints clone (bounded by the number of distinct cubes in the query, so small), plus the avoidable per-dimension work in finding 1. The push verbatim-repeat rule from the last round keeps hints_by_cube's per-cube lists from accumulating duplicate Vectors.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed. The fallback_hints_for_measure doc block now covers the fallback chain, the single-cube-view limitation, and the query-dependent meaning of COUNT(*) on a multi-fact view — all three are things a reader would otherwise have to reconstruct from the tests.

What I could not verify

I could not build or run anything — cargo check on cubesqlplanner is not permitted in this job's sandbox and this is a cold checkout with no target dir. Every finding is from static reading of the diff, the fixtures, the seed SQL and the collector; the snapshot arithmetic is hand-computed against integration_multi_fact_tables.sql. I also can't confirm that nested_root_view's non-root-anchored join paths compile cleanly through the mock view builder — that's a fixture shape the schema compiler wouldn't emit, so it's worth a glance at the test run. Prior finding 3 remains a request for confirmation, not a claimed regression.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[claude]

Claude finished @MazterQyou's task in 2m 22s —— View job

---

Re-review: both prior code findings fixed — 1 medium, 1 low (2 inline comments), 2 prior threads still open

Full review

• Gather context (diff vs origin/master, planner, fixtures, snapshots)
• Check which prior unresolved findings are addressed
• Review the new root/cyclic error split, cyclic_paths_view fixture, Option<HashMap> inheritance
• Re-verify the reachability rule against every fixture join map
• Resolve stale threads, post new findings

Prior findings

#	Prior finding	Resolution
1	Dimension/filter seeding of hints_by_cube was unreachable dead work	Seeding dropped entirely — hints_by_cube is now measures-only, and the doc says outright that dimensions/filters live in base_hints and what that implies. Thread resolved (see new finding 1 for the residual).
2	Ambiguous-root error couldn't name the conflicting cubes; cyclic map reported the wrong cause	roots is collected, the message lists them, and the empty case now has its own "join paths … are cyclic" error listing the paths. cyclic_paths_view + ..._with_cyclic_join_map cover it, and the ambiguous test asserts both root names appear. Thread resolved.
3	"pool" terminology now greps to nothing	Still open — integration_views.yaml:63 ("contributing to the pooled ones") and member_expressions.rs:244,278 still describe the removed pooled_hints. Not re-posted; the thread stands.
4	join_map in the mock changes hint collection for every view fixture — confirmation the suite is green	Still open. cargo is not permitted in this job either, so the ask stands.

What I verified

The root rule, hand-run against every join map the fixtures produce (mock keeps only len > 1 paths):

• customer_overview [[customers,orders],[customers,returns]] → reached={orders,returns} → sole root customers → snapshot 4 ✅
• nested_root_view [[returns,customers],[customers,orders]] → reached={customers,orders} → sole root returns ✅
• two_roots_view [[orders,customers],[returns,customers]] → reached={customers} → {orders, returns} → ambiguous error naming both ✅
• cyclic_paths_view [[orders,customers],[customers,orders]] → reached={customers,orders} → no head unreached → cyclic error ✅

roots.is_empty() is now genuinely equivalent to "every head is reached from another path", which over a finite set does mean a cycle — so the new error names the right cause rather than degrading into the generic one. join_map.is_empty() is short-circuited above it, so the single-cube-view limitation still lands on the caller's "nothing to join from" message, which is the accurate one there.

Also re-checked the earlier snapshot arithmetic survives the Option<HashMap> refactor unchanged: no_hints_on_view 2 | 8, no_hints_on_multi_fact_view 8 | 5 | 13, only_member_on_view_with_join_map 4, and the multi-stage pair. for_measures is still only handed subsets of the query's own measures, so inheriting the map wholesale stays sound.

New findings

#	Sev	Where	Issue
1	Medium	multi_fact_join_groups.rs:80-84	The same-view guard covers measures but not dimensions: COUNT(*) on orders_view + customer_overview.returns_count is now rejected, while the identical question with customer_overview.city as a dimension silently returns a customer count. Documented, but as a neutral detail rather than the known hole it is, and untested either way
2	Low	member_expressions.rs:367	sql.contains("returns") is the weakest of the new assertions for the subtlest rule; a paired negative would fail informatively if the root rule ever regresses

Neither blocks. Finding 1 is legacy parity and narrowing it properly means moving the check to where a hint-less view measure consumes base_hints, not to the seeding — a follow-up is a fine answer, but the doc wording and a locking test would be worth having now.

Code quality

Splitting the no-single-root case into "several roots" and "cyclic paths", with a shared no_single_root closure carrying the common prefix, is a nicer shape than a single message with a conditional tail — each arm states one fault and lists the evidence a user needs to fix the YAML. The comments above each arm explain why the arm exists rather than restating the match.

Dropping the dimension/filter seeding in favour of a plain Option<HashMap> is the right simplification over the HintsByCube enum: with only one producer left, the enum was carrying a distinction that no longer existed. The doc block picks up the invariant the enum used to encode and states it in the one place a reader looks.

The join_hints.rs doc — "a local rule, not a normal form", plus what unequal hints do and don't prove about the cache key — remains the clearest thing in the diff.

Testing

Nine tests now: three fallback branches, the cross-view rejection, the nested root, and three terminal errors (single-cube view, ambiguous roots, cyclic). The five error tests are sync #[test] and always assert, and the ambiguous/cyclic ones assert on message content, not just failure — good. Unchanged structural gap: the four positive tests only assert when a Postgres is reachable (try_execute_pg_from_options returns Option); without one they degrade to "the planner didn't panic".

Security / performance

No security surface — nothing user-controlled reaches SQL text through this path; the change only decides which cube hints seed join resolution. Performance is slightly better than the previous round: the per-dimension/per-filter hints_by_cube bookkeeping is gone, the inherited path still skips a symbol traversal per multi-stage measure, and the root computation is two small passes over a join map that is a handful of entries.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed.

What I could not verify

I could not build or run anything — cargo check on cubesqlplanner is not permitted in this job's sandbox and this is a cold checkout with no target dir. Every finding is from static reading of the diff, the fixtures and the seed SQL; the join-map arithmetic is hand-run. In particular I can't confirm that cyclic_paths_view and nested_root_view compile cleanly through the mock view builder — both are shapes the schema compiler wouldn't emit, so worth a glance at the test run. Prior findings 3 and 4 remain open asks, not claimed regressions.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[claude]

Claude finished @MazterQyou's task in 3m 55s —— View job

---

Re-review: both prior code findings addressed — 0 medium, 1 low (1 inline comment), 2 prior threads still open

Full review

• Gather context (diff, planner, fixtures, snapshots, seed data)
• Check which prior unresolved findings are addressed
• Review the new "known hole" documentation + ..._beside_other_view_dimension test
• Re-verify the root rule and snapshot arithmetic still hold
• Resolve stale threads, post new findings

Prior findings

#	Prior finding	Resolution
1	The same-view guard covers measures but not dimensions; documented as a neutral detail, untested	Now an explicit "Known hole, kept for legacy parity" paragraph on fallback_hints_for_measure (multi_fact_join_groups.rs:313-324) that names the mechanism (base_hints is not view-scoped), points at the pinning test, and says what closing it would cost. test_expr_measure_count_star_no_hints_beside_other_view_dimension locks the current behaviour in and says outright it flips to a rejection if the hole is ever closed. Thread resolved.
2	sql.contains("returns") was the weakest of the new assertions	Paired with !sql.contains("orders") and a message that names the actual fault ("a rule that took the first head instead of the unreached one would land on customers and pull orders in with it"). Thread resolved — see new finding for a residual on the sibling test that copies the idiom.
3	"pool" terminology greps to nothing now that pooled_hints is gone	Still open. integration_views.yaml:59-61 ("contributing to the pooled ones") and member_expressions.rs ("its hints still go into the pool", "The pooled hints are the union over both facts") still describe the removed field. Not re-posted; the thread stands.
4	join_map in the mock changes hint collection for every view fixture — confirmation the suite is green	Still open. cargo is not permitted in this job either, so the ask stands.

What I verified

The known-hole documentation matches the control flow exactly. base_hints absorbs dimension hints (multi_fact_join_groups.rs:48-50), mh.hints starts as a clone of it (:144), and fallback_hints_for_measure is only reached when mh.hints.is_empty() (:248) — so a hint-less view ME with any dimension in the query never reaches the view-scoped bucket. The new test's expectation (customers, no orders) is what that produces.

Re-ran the root rule against every fixture join map (the mock keeps only len > 1 paths), unchanged from the last round and still consistent with the tests:

• customer_overview [[customers,orders],[customers,returns]] → reached={orders,returns} → sole root customers → snapshot 4 ✅
• nested_root_view [[returns,customers],[customers,orders]] → reached={customers,orders} → sole root returns ✅
• two_roots_view [[orders,customers],[returns,customers]] → {orders, returns} → ambiguous error naming both ✅
• cyclic_paths_view [[orders,customers],[customers,orders]] → no unreached head → cyclic error ✅

The [] arm really does mean "cyclic": over a finite set, every head being reached from some other path's tail forces a cycle among the heads, so the message names the right cause rather than guessing.

Snapshot arithmetic is unaffected by this round — the grouping and the fallback chain are untouched — so 2 | 8, 8 | 5 | 13, 4 and the multi-stage pair all still follow from integration_multi_fact_tables.sql as computed in earlier rounds.

New finding

#	Sev	Where	Issue
1	Low	member_expressions.rs:397	!sql.contains("orders") passes only because the ME's owning view name never reaches the SQL; every other view member does get a cube prefix (many_to_one_view__root_dim), so an aliasing change would fail this test with "expected the hole to stand" while pointing at join resolution that didn't change

Doesn't block. Anchoring on the join shape (a customers-only plan emits no JOIN at all) instead of on substring absence would fail for the right reason.

Code quality

The known-hole paragraph is the best kind of comment for this: it states the hazard, names the mechanism that causes it, points at the test that pins it, and explains why closing it is a larger change than the fix at hand — including the non-obvious consequence that resolving from the view bucket whenever the measure's own hints are empty would make the ordinary case (a view dimension next to COUNT(*) on the same view) depend on that bucket carrying dimensions. A future fixer gets the whole picture without re-deriving it.

The new test's comment carries the same weight, and "if it is ever closed, this test flips to expecting the same rejection as the one below" is exactly the instruction that keeps a behaviour-pinning test from being mistaken for a behaviour-asserting one.

Everything else from the last round holds: the Option<HashMap> inheritance, the split root/cyclic errors with the shared no_single_root prefix, and the join_hints.rs doc's "a local rule, not a normal form".

Testing

Ten tests now: three fallback branches, the cross-view measure rejection, the cross-view dimension hole, the nested root, and three terminal errors. Six of them are sync #[test] and always assert, including all the error cases, which assert on message content rather than just failure. Unchanged structural gap: the four positive tests only assert when a Postgres is reachable (try_execute_pg_from_options returns Option); without one they degrade to "the planner didn't panic".

Security / performance

No security surface — nothing user-controlled reaches SQL text through this path; the change only decides which cube hints seed join resolution. Performance is unchanged from the last round (this round is documentation and a test); the per-cube map is one HashMap<String, JoinHints> clone per for_measures, bounded by the number of distinct cubes in the query.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed.

What I could not verify

I could not build or run anything — cargo is not permitted in this job's sandbox and this is a cold checkout with no target dir. Every claim above is from static reading of the diff, the fixtures, the seed SQL and the existing snapshots; the join-map arithmetic is hand-run. In particular I can't confirm that !sql.contains("orders") actually holds today (that's the substance of the new finding — it depends on member-expression aliasing, which I read from a snapshot rather than from the renderer), nor that cyclic_paths_view / nested_root_view compile cleanly through the mock view builder. Prior findings 3 and 4 remain open asks, not claimed regressions.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[claude]

Claude finished @MazterQyou's task in 1m 54s —— View job

---

Re-review: documentation-only round — 0 new findings, 2 prior threads still open (1 partially fixed)

Full review

• Gather context (current tree, planner, fixtures, tests, compiler source)
• Check which prior unresolved findings are addressed
• Verify the compiler-fidelity correction
• Resolve stale threads, decide on new findings

What changed this round

This push is comments and doc text only — multi_fact_join_groups.rs's control flow, join_hints.rs, the fixtures and the tests are byte-identical in behaviour to the round I reviewed at 16:01. So there is nothing new to re-verify on the join-resolution logic, the root rule or the snapshot arithmetic; all of that still holds as computed in the previous rounds.

Prior findings

#	Prior finding	Resolution
1	join_map in the mock changes hint collection for every view fixture — asked for confirmation the suite is green	Resolved. The reply gives the numbers: cargo test -p cubesqlplanner --lib 1222 passed / 0 failed; full pg-integration run with 10 failures that are all CREATE EXTENSION hll seed failures reproducing on master; 563 .snap files byte-identical with zero .snap.new; the specific suites I named (integration::views, the seven join_hints_collector view tests, all member_expressions_on_views) 26 passed / 0 failed. That is exactly the confirmation the thread asked for, so I've resolved it.
2	"pool" terminology greps to nothing now that pooled_hints is gone	Partially fixed — see below. Thread left open.
3	!sql.contains("orders") in ..._beside_other_view_dimension depends on ME aliasing, not on join shape	Not addressed. member_expressions.rs:402-404 still reads sql.contains("customers") && !sql.contains("orders"). Thread stands; not blocking.

On the fidelity correction

Checked CubeSymbols.prepareIncludes and the correction is right, my earlier nit was wrong. joinMap.push(split) sits at the top of the includedCubes.map(...) callback (CubeSymbols.ts:818-826), before it.includes is read at all — the type === 'dimensions' ternary only decides whether the pass runs, not which cubes are mapped. So a cube contributing no dimensions does get an entry, customer_overview (measures only from customers.orders) is mapped in production exactly as the mock maps it, and one-entry-per-view-cube is faithful. The rewritten comment in mock_schema.rs:647-661 records the real divergence instead — evaluateReferences vs a raw split('.'), so only a non-literal join path would differ — which is accurate and is the one a fixture author would actually trip over.

Terminology: one line left

Two of the three call-outs landed. ..._beside_multi_stage_measure now says "its hints still count towards its view's", and ..._on_multi_fact_view attributes the union to both facts being members of this view — both are more precise than what they replaced, and the surviving "pools" on multi_fact_join_groups.rs:313 genuinely describes the legacy planner, where it's correct.

The one still standing is the line the thread is anchored on, integration_views.yaml:61-63:

# Not multi-stage itself, but depends on a multi-stage measure, so it
# is skipped when building per-measure hints while still contributing
# to the pooled ones.

"the pooled ones" is the removed whole-query pooled_hints, not the legacy planner — the accurate phrasing is the one already used on the test that exercises this measure ("its view's"). Trivial, but it's the last grep hit for the old mechanism outside the legacy sentence.

Code quality

The expanded fallback_hints_for_measure doc block is now the best explanation of this mechanism anywhere in the tree. The addition that earns its place this round is the explicit contrast with legacy — "it borrows wider: it unions the join hints of every query member into one join tree, with no notion of which view a member came from. Narrowing that union to the measure's own view is the difference here." Previously the doc claimed parity with legacy while the code had deliberately diverged; now the divergence is the stated point, which is both truer and more useful.

The block is long (~40 lines) for one function, but each paragraph answers a question a reader will otherwise have: why a view can't seed itself, why other views aren't borrowed from, why the join map is the last resort and which views it misses, why the meaning is query-dependent, and where the dimension-shaped hole is. Splitting it wouldn't make it shorter, only harder to find.

Testing

Ten tests, unchanged from the last round: three fallback branches, the cross-view measure rejection, the cross-view dimension hole, the nested root, and three terminal errors. Six are sync #[test] and always assert; the four positive ones only assert when a Postgres is reachable (try_execute_pg_from_options returns Option) — structural, unchanged, and now partly answered by the confirmed full-suite run.

Security / performance

No change this round — the diff is comments. No security surface (nothing user-controlled reaches SQL text through this path), and the per-cube HashMap<String, JoinHints> clone per for_measures remains the only added cost.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed.

What I could not verify

I could not build or run anything — cargo is not permitted in this job's sandbox and this is a cold checkout with no target dir. The suite confirmation above is quoted from the reply, not independently reproduced. The compiler-fidelity check is from reading CubeSymbols.ts directly, which I could do. git fetch origin master is also unavailable here, so the "documentation-only" characterisation is from comparing the current file contents against my notes from the previous round rather than a commit-to-commit diff.

· branch [`tesseract/resolve-join-hint-views`](https://github.com/cube-js/cube/tree/tesseract/resolve-join-hint-views)

[claude]

Claude finished @MazterQyou's task in 1m 51s —— View job

---

Re-review: both prior threads addressed — 0 new findings, nothing outstanding

Full review

• Gather context (current tree, planner, fixtures, tests, snapshots)
• Check the two prior unresolved findings
• Verify the tightened assertions and the terminology fix
• Resolve stale threads, decide on new findings

Prior findings

#	Prior finding	Resolution
1	"pool" terminology greps to nothing now that pooled_hints is gone	Fixed. integration_views.yaml:60-62 now says the measure "is skipped when building per-measure hints while still counting towards the hints of the view it is included in" — accurate for hints_by_cube and it names the view scoping. A grep for pool across the planner, the fixture and the tests now returns exactly one hit, multi_fact_join_groups.rs:313, which describes the legacy planner and is correct there. Thread resolved.
2	!sql.contains("orders") depended on member-expression aliasing, not on join shape	Fixed, and on both tests. ..._beside_other_view_dimension (member_expressions.rs:411-414) is now sql.contains("FROM customers AS") && !sql.contains("JOIN"), and ..._with_nested_join_map (:377-380) got the same treatment with FROM returns AS. Both anchor on the plan shape — a single-cube plan emits no join — so a rename can't trip them, and the comment on the nested one names the exact regression it guards ("a rule that took the first head instead of the unreached one would root at customers and join orders onto it"). JOIN is uppercase in every join template (BaseQuery.js:1441,2039) and normalize_sql only collapses whitespace, so the negative is well-formed. Thread resolved.

What I re-verified

Both of these tests are sync #[test] over build_sql_from_options, so unlike the four snapshot tests they assert unconditionally — the hardening actually runs everywhere, which is what makes it worth doing. That brings the always-asserting count to seven of eleven; the remaining four (..._no_hints, ..._no_hints_on_view, ..._beside_multi_stage_measure, ..._on_multi_fact_view, ..._only_member_on_view_with_join_map) are the snapshot ones gated on a reachable Postgres — structural and unchanged.

Nothing else moved this round. The fallback chain in fallback_hints_for_measure, the hints_by_cube grouping, the root/cyclic split and join_hints.rs are byte-identical in behaviour to the round I reviewed at 16:21, so the join-map arithmetic (customer_overview → customers → 4; nested_root_view → returns; two_roots_view → ambiguous; cyclic_paths_view → cyclic) and the snapshot numbers (2 | 8, 8 | 5 | 13, 4, the multi-stage pair) all still follow as computed in the earlier rounds.

New findings

None. Both open threads are closed, and I found nothing new in a fresh read of multi_fact_join_groups.rs, the fixture and the tests.

Code quality

The final state reads well. The three things that carry the most weight for a future reader are all comments that explain a decision rather than restate code: the "Known hole, kept for legacy parity" paragraph on fallback_hints_for_measure, which names the mechanism (base_hints is not view-scoped), points at the pinning test and says what closing it would cost; the contrast with legacy ("it borrows wider … narrowing that union to the measure's own view is the difference here"), which replaced an earlier claim of parity that the code had deliberately diverged from; and join_hints.rs's "a local rule, not a normal form".

The one nit left over from several rounds ago still stands and still doesn't matter: the terminal error in build_groups:254-261 interpolates mh.measure.cube_name() twice, which reads repetitively when the referenced cube and the join-map owner are the same string. It is correct as written.

Testing

Eleven tests: three fallback branches, the cross-view measure rejection, the cross-view dimension hole, the nested root, the plain no-hints case, and three terminal errors. Seven always assert; the error cases assert on message content, not just failure.

Security / performance

No change this round. No security surface — nothing user-controlled reaches SQL text through this path; the change only decides which cube hints seed join resolution. The per-cube HashMap<String, JoinHints> clone per for_measures remains the only added cost, bounded by the number of distinct cubes in the query.

Docs

Nothing user-facing changed, so no /docs-mintlify update is needed.

What I could not verify

I could not build or run anything — cargo is not permitted in this job's sandbox and this is a cold checkout with no target dir. The assertions above are read statically; the JOIN casing claim is from the join templates in BaseQuery.js rather than from observed output. The full-suite confirmation from the earlier round (1222 lib tests, 563 byte-identical snapshots) is quoted from the author's reply, not independently reproduced. git fetch origin master is unavailable here, so "nothing else moved this round" is from comparing current file contents against my notes from the previous round rather than a commit-to-commit diff.

LGTM from my side — no blocking issues remain.
· branch tesseract/resolve-join-hint-views