fix(ingest/powerbi): resolve M-Query names against enclosing let scopes

[puneetagarwal-datahub]

Summary

The M-Query walk carried one let at a time. Descending into a nested let replaced the enclosing scope instead of adding to it, so a nested body could not reach any step its parents bound:

let
    Source = Databricks.Catalogs(...),
    db     = Source{[Name="cat", Kind="Database"]}[Data],
    sch    = db{[Name="sch", Kind="Schema"]}[Data],
    out    = let tbl = sch{[Name="tbl", Kind="Table"]}[Data] in tbl
in
    out

sch was unreachable from the inner let, so this table produced no lineage at all. Nested lets are common in Power BI-generated M and Source is the default step name, so this is not an exotic shape.

Approach

Carry the enclosing scopes as a chain, innermost last, and resolve a name from the inside out so the nearest binding wins. This replaces the current_let / current_let_id pair with a single scopes argument, so the walk signature gets shorter rather than longer.

The chain is truncated at the scope that binds the name. A step bound by an outer let is evaluated in that let's scope and cannot see names a nested one introduces. Without the truncation, this:

a   = sch{[Name="tbl", Kind="Table"]}[Data],          -- outer; `sch` unbound here
out = let sch = db{[Name="inner", Kind="Schema"]}[Data] in a

resolves sch against the nested binding and invents an upstream from an expression Power BI itself would reject. Verified by disabling the truncation: it emits cat.inner.tbl. That property is pinned by a test.

The circular-reference guard now keys on the scope that actually bound the name, rather than whichever let happened to be current.

Stacked on #19364

Base branch is #19364, not master.

This is not for convenience. #19364 adds a ParenthesizedExpression branch to the same walk, written against the old current_let / current_let_id pair. The two changes are textually compatible — git merges them without a conflict — and the result is broken:

_walk(node_map, node.get("content"), current_let, current_let_id, ...)   # names no longer exist

The resulting NameError is swallowed by the caller's broad except Exception, so it does not crash. It just returns no lineage, silently, for exactly the parenthesized shape #19364 exists to fix. Neither branch fails on its own; only the combination does.

Verified by merging the two and running the suite: test_databricks_parenthesized_navigation_step and ..._in_if_branch both fail with assert 0 == 1. This PR adapts that call site, which is why it stacks here rather than on master — CI now proves the combination before either lands.

Behaviour verified

shape	result
nested let reaching an outer step	resolves ✅ (was no lineage)
nested binding shadowing an outer name	innermost wins ✅
three levels of nesting	resolves ✅
outer step referencing a nested-only name	no lineage ✅ (does not fabricate)
a = a	no lineage, no recursion ✅
a = b, b = a	no lineage, no recursion ✅
sibling nested lets	no cross-leak ✅
parenthesized step, with #19364 in the base	resolves ✅ (fails without this PR)

Testing

373 passed, 2 xfailed · ruff, format and mypy clean · no existing goldens touched.

From review

• Circular-reference guard is casefolded. Identifier lookup compares case-insensitively because M variable names are, but the guard keyed on the name as written. Measured before the change: a = A and a = B, b = A both already terminated — each distinct spelling gets its own guard entry and spellings are finite — so this is a consistency fix, not a hang fix, and its test passes either way.
• M's exclusive identifier references are recorded, not fixed. A plain reference excludes the binding being initialized, so a nested a = a should read the enclosing a; resolution prefers the innermost binding, resolves it to itself, and the guard drops the lineage. Left as a strict xfail to keep this change to scope-chain resolution — no reported query uses the shape, and there are zero self-initialising steps in the reporting tenant's logged M. strict=True verified: applying the fix turns it XPASS and fails the run, so it cannot rot.

Note for reviewers

This is a change to resolution behaviour, not just to which expressions are recognised — worth the closer look of the M-Query PRs currently in flight. The swallowed-NameError path above is the part I would most like a second opinion on: a broad except Exception around the walk turns programming errors into missing lineage, and that is how both this and a second dropped-argument bug on #19372 stayed invisible. Narrowing it is out of scope here but worth its own change.

It also touches the same function as #19365; whichever merges second needs a rebase there. The conflict is mechanical (that PR renames the walk's accumulator, this one replaces its scope arguments) but it is not a one-liner.

Checklist

• PR conforms to the Contributing Guideline (particularly PR Title Format)
• Tests for the changes have been added
• Docs — n/a, no user-facing config change
• Breaking changes — none

---

Summary by cubic

Resolves M-Query identifier names against the full chain of enclosing let scopes so nested bodies can reach outer steps, while outer steps can’t see nested-only names. Also preserves scope through parenthesized expressions and tightens the circular-reference guard to be case-insensitive and keyed to the binding let.

• Restores lineage for nested lets with innermost-first resolution and truncates the scope chain at the binding scope.
• Prevents fabricated upstreams from outer steps resolving nested-only names.
• Casefolds the guard key to block mixed-case cycles and avoid extra traversals.
• Carries the scope through parenthesized expressions to avoid silent lineage loss.
• Records M’s exclusive identifier references as a strict xfail; nested “a = a” currently yields no lineage, and unresolvable self-inits also yield none.

^{Written for commit adaa416. Summary will update on new commits.}

[github-actions]

Linear: ING-3351

Thanks for your contribution! We have created an internal ticket to track this PR. A member of the core DataHub team will be assigned to review it within the next few business days - you will get a follow-up comment once a reviewer is assigned.

[cursor]

PR Summary

Overview
Fixes Power BI M-Query lineage when queries use nested let blocks. The AST walk no longer swaps a single “current let” for the inner scope; it carries a scope chain (outer → inner) and resolves identifiers innermost binding first via _resolve_in_scopes.

When continuing from a binding, the chain is truncated to the scope that defined the name, so outer steps cannot pick up variables only introduced in a nested let (avoiding fabricated upstreams). The circular-reference guard now keys on the binding let id and a casefolded variable name so mixed-case cycles are treated as one visit.

Adds integration tests for nested outer access, shadowing, outer-vs-nested visibility, cycles, and unresolvable self-init; one xfail remains for nested a = a (M’s exclusive self-reference semantics).

^{Reviewed by Cursor Bugbot for commit adaa416. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...tahub/ingestion/source/powerbi/m_query/resolver.py	78.57%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

[datahub-connector-tests]

Connector Tests Results

All connector tests passed for commit adaa416

View full test logs →

To skip connector tests, add the skip-connector-tests label (org members only).

Autogenerated by the connector-tests CI pipeline.

[alfiyas-datahub]

Make the circular-reference guard case-insensitive (casefold()), matching how identifiers are looked up, to prevent loops when the same variable is written with different casing.

[puneetagarwal-datahub]

Done in 4aa549e — you're right that the guard was inconsistent with the lookup. resolve_identifier compares case-insensitively (its own comment says "M-Query variable names are case-insensitive") while the guard keyed on the name as written.

One thing worth being straight about: I went looking for the loop and could not produce one. Measured before the change:

expression	result
a = A (self-ref, different spelling)	guard fired, no lineage
a = B, b = A (cycle, mixed spelling)	guard fired, no lineage
a = b, b = a (same spelling)	guard fired, no lineage

All terminated. The reason is that each distinct spelling gets its own guard entry and an expression contains finitely many spellings, so the guard still fires — just one traversal later than it should. So this is a consistency fix rather than a hang fix, and the test I added passes with or without it. I'd rather say that than imply I closed a live bug.

Still worth making the change: keying the guard on the same normalisation the lookup uses removes a discrepancy that would bite the first time something else depends on the guard firing promptly.

[alfiyas-datahub]

The truncation behavior is correct, but we should clarify one edge case: how should a nested a = a be resolved when a is also defined in the outer scope?

[puneetagarwal-datahub]

Good question, and the answer needed pinning down rather than reasoning about — added test_nested_self_reference_does_not_fall_back_to_the_outer_binding in 4aa549e.

let
    Source = Databricks.Catalogs(...),
    a      = Source{[Name="my_catalog", Kind="Database"]}[Data],   -- outer `a`, a real navigation
    out    = let a = a in a                                        -- nested `a = a`
in
    out

Result: no lineage. Which is the behaviour we want, for two reasons.

The nested binding shadows the outer one for the whole nested body, so the right-hand a is the inner a — a genuine self-reference, which is what Power BI itself rejects as a cyclic reference. So the walk resolves a to the inner binding, the guard fires on (inner let id, "a"), and the branch stops.

The alternative — falling through to the outer a after the self-reference is detected — would emit my_catalog as an upstream for an expression Power BI will not even evaluate. That is the same failure mode the truncation exists to prevent, just reached from the other direction. Measured: 0 data-access functions found, and the outer a does point at a real navigation, so if it were falling back we would see 1.

Worth noting the guard is what makes this safe, not the truncation — the truncation governs which scopes a resolved value is evaluated in, while this case is decided by the guard firing on the inner binding. Two separate mechanisms, which is why it deserved its own test rather than being assumed covered.

[puneetagarwal-datahub]

Correction — my answer above was wrong, and Bugbot caught it on the same commit.

I said the nested a = a is a self-reference that Power BI rejects, and that resolving it against the outer binding would invent lineage. The opposite is true. From the M spec:

the let-expression evaluates the sub-expression for each variable with an environment containing each of the variables of the let except the one being initialized

with this example:

[
    a = [
        x = 1,       // environment: b, x (outer), y, z
        ...

For the binding being initialized, its own name is excluded and the enclosing one is in scope. So a = a reads the outer a — valid M that Power BI refreshes — and is only an error when nothing encloses it. A plain reference is exclusive; @a is the inclusive form.

So the behaviour I described as correct was a lineage-loss bug: the guard fired on a reference that was never circular. Fixed in 21e289c — resolution now skips a binding while its own value is being walked. The test I pointed you at is replaced by two, one per case, and the positive one fails if the exclusion is removed.

Your original question was the right one to ask; I answered it confidently from the wrong model of M's scoping instead of checking the spec. Thanks for pushing on it.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 4aa549e. Configure here.}

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="metadata-ingestion/src/datahub/ingestion/source/powerbi/m_query/resolver.py">

<violation number="1" location="metadata-ingestion/src/datahub/ingestion/source/powerbi/m_query/resolver.py:378">
P1: Custom agent: **Enforce Strict Maintainability Standards**

When an inner same-name cycle exists alongside an outer binding, this lookup skips the inner binding because `seen` retains historical visits, then resolves the outer binding and emits fabricated lineage. Track only active bindings for recursion, or preserve the inner binding and stop the cycle without falling through to the outer scope.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Custom agent: Enforce Strict Maintainability Standards

When an inner same-name cycle exists alongside an outer binding, this lookup skips the inner binding because seen retains historical visits, then resolves the outer binding and emits fabricated lineage. Track only active bindings for recursion, or preserve the inner binding and stop the cycle without falling through to the outer scope.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At metadata-ingestion/src/datahub/ingestion/source/powerbi/m_query/resolver.py, line 378:

<comment>When an inner same-name cycle exists alongside an outer binding, this lookup skips the inner binding because `seen` retains historical visits, then resolves the outer binding and emits fabricated lineage. Track only active bindings for recursion, or preserve the inner binding and stop the cycle without falling through to the outer scope.</comment>

<file context>
@@ -353,17 +353,30 @@ def _unwrap_csv(elem: object) -> Optional[dict]:
     """
     for index in range(len(scopes) - 1, -1, -1):
         let_id, let_node = scopes[index]
+        if (let_id, name.casefold()) in initializing:
+            continue
         resolved = resolve_identifier(node_map, let_node, name)
</file context>