Add downstream lineage command for warehouse tables - #46
Conversation
📝 WalkthroughWalkthroughAdds dependency client APIs and lineage traversal utilities, then registers a ChangesLineage descendants
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant MetabaseClient
participant DependencyAPI
participant LineageWalker
participant OutputView
CLI->>MetabaseClient: resolve matching physical table
CLI->>DependencyAPI: fetch dependents and backfill status
DependencyAPI-->>CLI: return dependency nodes and status
CLI->>LineageWalker: walk descendants
LineageWalker-->>CLI: return descendant records
CLI->>OutputView: render filtered list
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/cli/src/core/lineage.ts (1)
40-45: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid quadratic queue dequeues.
Array.shift()reindexes all remaining entries on every visit, making a wide dependency graph O(V²). Track a queue cursor instead.Proposed fix
const queue: Array<{ entity: EntityRef; path: EntityRef[] }> = [{ entity: root, path: [root] }]; +let cursor = 0; -while (queue.length > 0) { - const current = queue.shift(); +while (cursor < queue.length) { + const current = queue[cursor]; + cursor += 1; if (current === undefined) { - break; + continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/core/lineage.ts` around lines 40 - 45, Replace the Array.shift() dequeue in the lineage traversal with a cursor-based queue index, advancing the cursor as each queued item is processed. Keep the existing queue initialization, seen tracking, and descendant traversal behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/core/lineage.test.ts`:
- Around line 19-30: The mock read function should throw when a dependency graph
key is not defined instead of treating it as an empty leaf. Update the graph
lookup in the read function so known keys still return their arrays, while
unknown `${type}:${id}` keys fail explicitly.
In `@packages/cli/src/core/lineage.ts`:
- Around line 35-62: Thread the command’s interruptSignal through the
descendants flow: update DependencyReader and walkDescendants to accept and
forward it on every dependency read, then update
packages/cli/src/commands/lineage/descendants.ts lines 41-78 to pass the signal
to client construction, client.database.list(), schemaTables(),
walkDescendants(), and client.dependency.backfillStatus().
In `@packages/cli/src/output/views/descendant.ts`:
- Around line 28-43: Update the path projection/schema for DescendantCompact to
reject malformed lineage entries instead of filtering them out. Change the path
column’s format callback to accept the schema-inferred EntityRef type and map
validated references directly to their type/id representation, preserving the
existing arrow-separated output without the unknown-value fallback.
In `@packages/client/src/domain/dependency.ts`:
- Around line 19-28: Add exported compact schemas for the dependency domain
alongside DependencyNode and DependencyBackfillStatus, using pick(...).strip()
to define the endpoint-specific compact shapes. Ensure each compact schema
includes only the fields required by its resource and preserves the existing
full schemas and inferred types.
- Around line 19-23: Replace the unvalidated data record in DependencyNode with
named schemas for each dependency entity/card type, using a discriminated union
keyed by type and validating the corresponding card fields and type values.
Reuse the existing DependencyEntityType definitions where applicable, and keep
dependents_count validation unchanged.
In `@packages/client/src/resources/dependency.ts`:
- Around line 10-22: Update dependencyResource’s dependents method in
packages/client/src/resources/dependency.ts lines 10-22 to parse the private
wire envelope and return the consistent ListResult<DependencyNode> domain shape
instead of a raw array. Update packages/client/src/resources/dependency.test.ts
lines 28-40 to use an enveloped fixture and assert the corresponding ListResult
result.
---
Nitpick comments:
In `@packages/cli/src/core/lineage.ts`:
- Around line 40-45: Replace the Array.shift() dequeue in the lineage traversal
with a cursor-based queue index, advancing the cursor as each queued item is
processed. Keep the existing queue initialization, seen tracking, and descendant
traversal behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 75db681d-6204-47df-815f-5dbbff53d3bf
📒 Files selected for processing (11)
packages/cli/src/commands/lineage/descendants.tspackages/cli/src/commands/lineage/index.tspackages/cli/src/core/lineage.test.tspackages/cli/src/core/lineage.tspackages/cli/src/main.tspackages/cli/src/output/views/descendant.tspackages/cli/src/runtime/command-help.test.tspackages/client/src/client.tspackages/client/src/domain/dependency.tspackages/client/src/resources/dependency.test.tspackages/client/src/resources/dependency.ts
| const read = vi.fn(async (type: DependencyNode["type"], id: number) => { | ||
| const graph: Record<string, DependencyNode[]> = { | ||
| "table:1": [ | ||
| node(2, "card", { type: "model", name: "Orders model" }), | ||
| node(3, "card", { name: "Question" }), | ||
| ], | ||
| "card:2": [node(4, "dashboard", { name: "Operations" })], | ||
| "card:3": [node(4, "dashboard", { name: "Operations" })], | ||
| "dashboard:4": [node(2, "card", { type: "model", name: "Orders model" })], | ||
| }; | ||
| return graph[`${type}:${id}`] ?? []; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make unexpected graph reads fail.
?? [] turns an unmodeled dependency read into a leaf, which can hide traversal regressions. Throw for unknown graph keys instead.
As per coding guidelines, “Do not use placeholder fallbacks such as ?? "", ?? 0, ?? [], or ?? {} when the semantic state is missing.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/core/lineage.test.ts` around lines 19 - 30, The mock read
function should throw when a dependency graph key is not defined instead of
treating it as an empty leaf. Update the graph lookup in the read function so
known keys still return their arrays, while unknown `${type}:${id}` keys fail
explicitly.
Source: Coding guidelines
| export interface DependencyReader { | ||
| (type: EntityRef["type"], id: number): Promise<DependencyNode[]>; | ||
| } | ||
|
|
||
| export async function walkDescendants(read: DependencyReader, root: EntityRef) { | ||
| const queue: Array<{ entity: EntityRef; path: EntityRef[] }> = [{ entity: root, path: [root] }]; | ||
| const seen = new Set([entityKey(root)]); | ||
| const descendants: Descendant[] = []; | ||
|
|
||
| while (queue.length > 0) { | ||
| const current = queue.shift(); | ||
| if (current === undefined) { | ||
| break; | ||
| } | ||
| const direct = await read(current.entity.type, current.entity.id); | ||
| for (const node of direct) { | ||
| const entity: EntityRef = { type: node.type, id: node.id }; | ||
| const key = entityKey(entity); | ||
| if (seen.has(key)) { | ||
| continue; | ||
| } | ||
| seen.add(key); | ||
| const path = [...current.path, entity]; | ||
| descendants.push(descendantFrom(node, path)); | ||
| queue.push({ entity, path }); | ||
| } | ||
| } | ||
| return descendants; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline packages/cli/src/commands/runtime.ts --items all
ast-grep outline packages/client/src/resources/dependency.ts --items all
rg -n -C2 'interruptSignal|dependents|backfillStatus|schemaTables' \
packages/cli/src/commands/runtime.ts \
packages/client/src/resources/dependency.tsRepository: metabase/metabase-cli
Length of output: 4653
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lineage files =="
fd -a 'lineage\.ts|descendants\.ts|runtime\.ts|transport\.ts|client\.ts' packages | sed 's#^\./##' | sort
echo "== relevant lineage snippets =="
cat -n packages/cli/src/core/lineage.ts | sed -n '1,120p'
echo "== descendants command =="
cat -n packages/cli/src/commands/lineage/descendants.ts | sed -n '1,140p'
echo "== runtime command context =="
cat -n packages/cli/src/commands/runtime.ts | sed -n '60,135p'
echo "== dependency resource full =="
cat -n packages/client/src/resources/dependency.ts | sed -n '1,120p'
echo "== interface/search for getClients/Client construction =="
rg -n -C3 'getClient|new MetabaseClient|MetabaseClient\(|transport\.|RequestOptions|signal|interruptSignal|database\.list|database\.schemaTables' packages/cli packages/client/src -g '*.ts' | sed -n '1,240p'Repository: metabase/metabase-cli
Length of output: 29174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== client constructor options =="
cat -n packages/client/src/client.ts | sed -n '1,180p'
echo "== database resource files =="
fd -a 'database|listAll|schemaTables' packages/client packages/cli -t f | sort
for f in packages/client/src -type f 2>/dev/null; do :; done # placeholder to stop fd warning
fd -a 'database\.ts$' packages/client packages/cli | sort | xargs -r -I{} sh -c 'echo "===== {} ====="; cat -n "{}" | sed -n "1,180p"'
echo "== transport signal implementation =="
cat -n packages/client/src/http/transport.ts | sed -n '1,220p'
echo "== signal helpers =="
cat -n packages/client/src/signal.ts | sed -n '1,180p'
echo "== lineages imports/commands =="
fd -a '.*lineage.*|lineage' packages/cli -t d -t f | sort
for f in $(fd -a '.*lineage.*|lineage' packages/cli -t f | sort); do echo "===== $f ====="; sed -n '1,160p' "$f"; doneRepository: metabase/metabase-cli
Length of output: 4521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== transport implementation =="
cat -n packages/client/src/http/transport.ts | sed -n '1,240p'
echo "== signal helpers =="
cat -n packages/client/src/signal.ts | sed -n '1,180p'
echo "== database resource implementation =="
cat -n packages/client/src/resources/database.ts | sed -n '1,160p'
echo "== transport tests relevant to client signal merge =="
cat -n packages/client/src/http/transport.test.ts | sed -n '360,450p'
cat -n packages/client/src/http/transport.test.ts | sed -n '440,500p'
echo "== lineage command imports =="
find packages/cli/src -path '*lineage*' -type f | sort | while read -r f; do
echo "===== $f ====="
sed -n '1,170p' "$f"
doneRepository: metabase/metabase-cli
Length of output: 33154
Thread cancellation through the descendants flow.
walkDescendants() accepts an unbounded unbounded reader contract but has nowhere to carry cancellation into each dependency read, and the command passes no explicit signal to these blocking operations.
packages/cli/src/core/lineage.ts#L35-L62: addinterruptSignaltoDependencyReaderandwalkDescendants, and forward it on each read.packages/cli/src/commands/lineage/descendants.ts#L41-L78: pass the command signal toclient.database.list(),schemaTables(),walkDescendants(), andclient.dependency.backfillStatus().
As per coding guidelines, operations that can block must accept interruptSignal explicitly, including client construction and long-running fetches.
📍 Affects 2 files
packages/cli/src/core/lineage.ts#L35-L62(this comment)packages/cli/src/commands/lineage/descendants.ts#L41-L78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/core/lineage.ts` around lines 35 - 62, Thread the command’s
interruptSignal through the descendants flow: update DependencyReader and
walkDescendants to accept and forward it on every dependency read, then update
packages/cli/src/commands/lineage/descendants.ts lines 41-78 to pass the signal
to client construction, client.database.list(), schemaTables(),
walkDescendants(), and client.dependency.backfillStatus().
Source: Coding guidelines
| format: (value) => | ||
| Array.isArray(value) | ||
| ? value | ||
| .filter((item): item is { type: string; id: number } => | ||
| Boolean( | ||
| typeof item === "object" && | ||
| item !== null && | ||
| "type" in item && | ||
| "id" in item && | ||
| typeof item.type === "string" && | ||
| typeof item.id === "number", | ||
| ), | ||
| ) | ||
| .map((item) => `${item.type}:${item.id}`) | ||
| .join(" -> ") | ||
| : "", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline packages/cli/src/output/views/descendant.ts --view expanded || true
echo "== descendant.ts =="
cat -n packages/cli/src/output/views/descendant.ts
echo "== nearby schema definitions/usages =="
rg -n "EntityRef|descendantView|format:" packages/cli/src packages/client/src -g '*.ts' -g '*.tsx' | head -200Repository: metabase/metabase-cli
Length of output: 12259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ResourceView and compactPick handling =="
cat -n packages/cli/src/output/types.ts | sed -n '1,150p'
echo "== renderList compactPick integration =="
rg -n "compactPick|ResourceView|renderList" packages/cli/src/output -g '*.ts' -C 4
echo "== lineage Descendant interface =="
cat -n packages/cli/src/core/lineage.ts | sed -n '1,120p'Repository: metabase/metabase-cli
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== compactPick safeParse path =="
sed -n '50,62p' packages/cli/src/output/projection.ts
echo "== projection fallback and column formatting =="
sed -n '42,95p' packages/cli/src/output/projection.ts
sed -n '1,95p' packages/cli/src/output/table.ts
echo "== dependency types =="
cat -n packages/client/src/domain/dependency.ts | sed -n '1,120p'Repository: metabase/metabase-cli
Length of output: 5323
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== projection pickPath projection and column type usage =="
sed -n '62,180p' packages/cli/src/output/projection.ts
echo "== read-only semantic verifier for formatter behavior =="
node - <<'JS'
function currentFormat(value) {
return Array.isArray(value)
? value
.filter((item) =>
Boolean(
typeof item === "object" &&
item !== null &&
"type" in item &&
"id" in item &&
typeof item.type === "string" &&
typeof item.id === "number",
),
)
.map((item) => `${item.type}:${item.id}`)
.join(" -> ")
: "";
}
for (const value of [
null,
undefined,
123,
[{ type: "card", id: 42 }],
[{ type: "card", id: 42 }, { type: 123, id: 7 }, { type: null, id: 8 }],
]) {
console.log(JSON.stringify(value), "=>", JSON.stringify(currentFormat(value)));
}
JSRepository: metabase/metabase-cli
Length of output: 1845
Fail on malformed lineage paths instead of dropping entries.
The path column is part of DescendantCompact, but this formatter still accepts any unknown legacy value and silently truncates malformed items to "". Make the projection/schema path fail first, and use the inferred EntityRef type for the formatter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/output/views/descendant.ts` around lines 28 - 43, Update the
path projection/schema for DescendantCompact to reject malformed lineage entries
instead of filtering them out. Change the path column’s format callback to
accept the schema-inferred EntityRef type and map validated references directly
to their type/id representation, preserving the existing arrow-separated output
without the unknown-value fallback.
Source: Coding guidelines
| export const DependencyNode = z.object({ | ||
| id: z.number().int().positive(), | ||
| type: DependencyEntityType, | ||
| data: z.record(z.string(), z.unknown()), | ||
| dependents_count: z.record(z.string(), z.number().int().nonnegative()).nullable().optional(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'DependencyNode|DependencyCardType|\.data\b' packages/client/src packages/cli/srcRepository: metabase/metabase-cli
Length of output: 32713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dependency domain =="
cat -n packages/client/src/domain/dependency.ts
echo "== dependency resource =="
cat -n packages/client/src/resources/dependency.ts
echo "== dependency-related files =="
fd -i 'dependency' packages/client packages/cli -t f | sed -n '1,120p'
echo "== dependency tests/usages =="
rg -n -C 2 'dependents|dependencyResource|DependencyNodeList|DependencyNode' packages/client/src packages/cli/src tests -g '!node_modules' || true
echo "== behavioral schema probe =="
node - <<'JS'
try {
const z = require('zod');
const schema = z.object({
id: z.number().int().positive(),
type: z.enum(['card', 'table', 'dataset', 'database', 'metric', 'model', 'question']),
data: z.record(z.string(), z.unknown()),
});
const payload = {
id: 1,
type: 'card',
data: { fakeField: true, arbitraryNested: { a: 1 }, cardType: 'banana' },
};
console.log('record unknown accepts arbitrary payload:', schema.safeParse(payload).success);
const union = z.discriminatedUnion('type', [
z.object({ type: z.literal('card'), cardType: z.enum(['question', 'model', 'metric']), id: z.number(), name: z.string() }),
z.object({ type: z.literal('table'), id: z.number(), name: z.string() }),
]);
console.log('discriminated union rejects wrong card type:', !union.safeParse({ type: 'card', cardType: 'banana' }).success);
console.log('discriminated union rejects extra unknown card field:', !union.safeParse({ type: 'card', cardType: 'question', fakeField: true }).success);
} catch (error) {
if (error.code === 'MODULE_NOT_FOUND') {
console.log('zod not available');
} else {
console.log(error);
}
}
JSRepository: metabase/metabase-cli
Length of output: 11029
Parse dependency payloads with named schemas.
data: z.record(z.string(), z.unknown()) lets /api/ee/dependencies/graph/dependents responses pass without validating card fields or the type/card-type values. Define named dependency-node data schemas, preferably a discriminated union on type, instead of Record<string, unknown>.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/client/src/domain/dependency.ts` around lines 19 - 23, Replace the
unvalidated data record in DependencyNode with named schemas for each dependency
entity/card type, using a discriminated union keyed by type and validating the
corresponding card fields and type values. Reuse the existing
DependencyEntityType definitions where applicable, and keep dependents_count
validation unchanged.
Source: Coding guidelines
| export const DependencyNode = z.object({ | ||
| id: z.number().int().positive(), | ||
| type: DependencyEntityType, | ||
| data: z.record(z.string(), z.unknown()), | ||
| dependents_count: z.record(z.string(), z.number().int().nonnegative()).nullable().optional(), | ||
| }); | ||
| export type DependencyNode = z.infer<typeof DependencyNode>; | ||
|
|
||
| export const DependencyBackfillStatus = z.object({ complete: z.boolean() }); | ||
| export type DependencyBackfillStatus = z.infer<typeof DependencyBackfillStatus>; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'z\.looseObject|export const .*Compact|\.pick\(' packages/client/src/domainRepository: metabase/metabase-cli
Length of output: 15588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "dependency.ts outline:"
ast-grep outline packages/client/src/domain/dependency.ts || true
echo
echo "dependency.ts content:"
cat -n packages/client/src/domain/dependency.ts
echo
echo "imports/usages of dependency domain:"
rg -n 'Dependency|dependency' packages/client/src packages/cli 2>/dev/null || trueRepository: metabase/metabase-cli
Length of output: 6999
Export the required dependency domain compact schemas.
packages/client/src/domain/dependency.ts exports DependencyNode and DependencyBackfillStatus without matched <Resource>Compact schemas. Add .pick(...).strip() exports for the endpoint data shapes this resource uses, or split the contracts if full/compact payloads differ.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/client/src/domain/dependency.ts` around lines 19 - 28, Add exported
compact schemas for the dependency domain alongside DependencyNode and
DependencyBackfillStatus, using pick(...).strip() to define the
endpoint-specific compact shapes. Ensure each compact schema includes only the
fields required by its resource and preserves the existing full schemas and
inferred types.
Source: Coding guidelines
| const DependencyNodeList = z.array(DependencyNode); | ||
|
|
||
| export function dependencyResource(transport: Transport) { | ||
| /** List the direct dependents of an entity. */ | ||
| async function dependents( | ||
| type: DependencyEntityType, | ||
| id: number, | ||
| options: RequestOptions = {}, | ||
| ): Promise<DependencyNode[]> { | ||
| return transport.requestParsed(DependencyNodeList, "/api/ee/dependencies/graph/dependents", { | ||
| ...options, | ||
| query: { type, id, "include-personal-collections": true }, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use one consistent ListResult<DependencyNode> contract.
The resource currently returns a raw array, and the test cements that shape. Change both sides together:
packages/client/src/resources/dependency.ts#L10-L22: parse the private wire envelope and returnListResult<DependencyNode>.packages/client/src/resources/dependency.test.ts#L28-L40: update the fixture and assertion to match the envelope/domain result.
📍 Affects 2 files
packages/client/src/resources/dependency.ts#L10-L22(this comment)packages/client/src/resources/dependency.test.ts#L28-L40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/client/src/resources/dependency.ts` around lines 10 - 22, Update
dependencyResource’s dependents method in
packages/client/src/resources/dependency.ts lines 10-22 to parse the private
wire envelope and return the consistent ListResult<DependencyNode> domain shape
instead of a raw array. Update packages/client/src/resources/dependency.test.ts
lines 28-40 to use an enveloped fixture and assert the corresponding ListResult
result.
Source: Coding guidelines
a5f01fd to
c6fb5b6
Compare
Summary
lineage dependentscommand for finding Metabase entities downstream of a physical warehouse tableValidation