Add diffview in web server#3
Merged
Merged
Conversation
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io> Co-authored-by: Wei-Chun, Chang <wcchang@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io> Co-authored-by: Wei-Chun, Chang <wcchang@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io> Co-authored-by: Wei-Chun, Chang <wcchang@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
popcornylu
force-pushed
the
feature/add-web-diff-view
branch
from
50744af to
dc3f18e
Compare
Signed-off-by: popcorny <celu@infuseai.io>
Signed-off-by: popcorny <celu@infuseai.io>
iamcxa
added a commit
that referenced
this pull request
Feb 6, 2026
Changes based on Copilot review: 1. Add PERMISSION_DENIED status code to distinguish authorization errors from missing table errors (#5) 2. Add defensive check for node.config.materialized is not None to handle edge cases (#2) 3. Remove unnecessary raw string prefix from SQL template (#3) 4. Fix SQLMesh initialization - only set TABLE_NOT_FOUND after query actually fails, not as misleading default (#7) 5. Add test for PERMISSION_DENIED status Skipped suggestions: - #1: Integration tests (better as separate PR) - #4: Logger convention (__name__ is standard Python) - #6: Error pattern specific enough for count(*) context Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Kent <iamcxa@gmail.com>
2 tasks
2 tasks
even-wei
added a commit
that referenced
this pull request
May 25, 2026
Backend ChangeStatus now emits "unknown" via _unresolvable_cte_change fallback (recce/util/breaking.py:277-280) and dbt_adapter propagation. Frontend wire-format types were narrower than what the backend serializes, so values are stripped on the type boundary and papered over with `as` casts downstream. Addresses Andy's review on 37b2b43: - info.ts:10 NodeColumnData.change_status (Finding #2) - info.ts:81 MergedNodeData.change_status (sibling of #3 at merged-node level) - info.ts:84 MergedNodeData.change.columns (Finding #1) - cll.ts:37 CllNodeData.change_status (Finding #3) - patchLineageDiffFromCll.ts:28,30 column-change locals (consequence of widening #2) Branch C reachability (Notes) is left as documented dead-but-defensive per the test pin in 37b2b43 — handled as a separate follow-up. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io>
5 tasks
even-wei
added a commit
that referenced
this pull request
May 25, 2026
…DRC-3411) Addresses PR #1376 review (Andy, Issues #3 and #5). Issue #3: when `prev` was undefined, `onCancel` seeded the React Query cache with `{ run_id, status: "Cancelled", type: "unknown" } as unknown as Run`. `"unknown"` is not a member of `RunType`, and the double-cast disabled every downstream type check (`findByRunType`, `isQueryRun`, `RunResultView` selection). The synthetic write was only safe because `RunView` already short-circuits on the `useCanceledRuns` sticky set and `isRunning=false`. Any future consumer reading `run.type` ahead of that gate would see meaningless data. Fix: when there's no prior cached Run, leave the cache empty. The sticky set + `isRunning=false` already cover the UI gates on every surface that renders a cancelled run (`RunView`, `RunResultPane`, `RunStatusAndDateDisplay`). Issue #5: `useCallback` deps listed the entire `canceledRuns` object, which `useCanceledRuns` returns as a fresh literal every render. That recreated `onCancel` on every render, defeating memoization. Pull out `canceledRuns.add` (already stable via `useCallback([])`) into a local and depend on it instead. Signed-off-by: even-wei <evenwei@infuseai.io>
2 tasks
2 tasks
even-wei
added a commit
that referenced
this pull request
May 25, 2026
…lved (#1375) * fix(breaking): surface unknown when CTE-internal change can't be resolved When parse_change_category runs without a parent schema (e.g., catalog shipped with missing source columns from a CI race), sqlglot's qualify is skipped and column refs through CTEs don't carry table qualifiers. The analyzer used to silently return change.columns={} and category non_breaking, causing the Columns tab to show no marker while the column-level lineage view loudly emitted "unknown" — the two surfaces disagreed. Detect the unresolvable state in _diff_select_scope and propagate "unknown" per-column and at the category level so the Columns tab can render a "?" badge that matches the CLL view. Resolves DRC-3409. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * fix(breaking): surface unknown per-column in mixed-edit models The previous guard's `not changed_columns` clause only fired in all-or-nothing cases. Models with both resolved outer changes (e.g., an added column) and an unresolvable CTE-internal change silently dropped the unresolvable half — same shape as the original DRC-3409 bug at the per-column level. Track which source-column changes the per-projection analysis actually traced; treat any unobserved CTE *modification* as an unresolvable signal. When triggered, every uncovered outer projection surfaces as "unknown" alongside the resolved ones, and a pure non_breaking category escalates to "unknown" so callers don't treat it as risk-free. Partial/breaking categories stay loud — the unknown columns ride alongside. Also fixes "unknown" not being counted in the dashboard column-change summary (calculateChangeSummary) and the storybook visual-regression fixture (isRowChanged) so a model showing the new "?" badge renders with the changed-row background. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * docs(drc-3409): add verification screenshots Captures pre-fix vs post-fix UI state on the synthetic jaffle_shop-expand fixture (model stg_orders_with_year, column location_year). Five screenshots covering: - SS-1/SS-2: pre-fix (main @ 2fc9ef2) Columns tab + CLL view - SS-3/SS-4: post-fix (PR HEAD @ d892c06) Columns tab + CLL view - SS-5: post-fix paired Columns + CLL view (surface agreement state) Note: the synthetic fixture ships a fully populated catalog and does not reproduce the original Paradime CI-race "missing source columns" condition, so the PR's new "?" badge does not visually trigger here. Visible delta is the legend addition (Unknown 'U' badge in the CLL canvas legend) and the absence of the "Definition changed" tooltip suffix on the post-fix CLL column row. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * docs(drc-3409): replace baseline-behavior screenshots with degraded-catalog before/after Captures real bug -> fix transition on a degraded-catalog fixture variant (pr-19-cte-case-when-degraded) that reproduces the Paradime CI-race condition behind DRC-3409. The previous fixture ships a fully populated catalog and never triggers the parse_change_category fallback path the PR adds. Degradation: target/catalog.json has source.jaffle_shop.ecom.raw_orders removed, leaving its column metadata empty. With no source-column schema, sqlglot qualify() cannot attach table qualifiers to CTE-internal column refs, and the analyzer falls back to "unknown". Screenshots: - SS-1 BEFORE Columns tab: empty column list, canvas labels model "Non Breaking" (the bug) - SS-2 BEFORE CLL view: sidebar marks model Modified while canvas shows Non Breaking — surfaces disagree - SS-3 AFTER Columns tab: every column row shows the new "?" badge - SS-4 AFTER CLL view: canvas labels model "Unknown" and surfaces now agree - SS-5 AFTER paired agreement: Columns tab + canvas both show "?"/Unknown for location_year and siblings Tested: - uv run pytest tests/util/test_breaking.py -> 52 passed - cd js && pnpm vitest run -> 3719 passed, 5 skipped Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * test(breaking): pin DISTINCT-in-CTE behavior; document branch C gap Reviewer asked for a test covering the `category == "breaking" and not src_change.columns` branch of `_unresolvable_cte_change`. The branch is not reachable in current code paths: the upstream propagation in `_diff_select_scope` (lines 75-79) sets the outer scope to "breaking" whenever a source CTE is "breaking", and the guard at line 277 (`change_category != "breaking"`) then bypasses the loud-fail. DISTINCT-in-CTE — the reviewer's suggested scenario — therefore surfaces as `category="breaking", columns={}` rather than per-column "unknown". This test pins that behavior so a follow-up that broadens the loud-fail to tag columns even when the outer is already "breaking" will visibly flip the assertion. Refs DRC-3409. Signed-off-by: even-wei <evenwei@infuseai.io> * fix(types): widen wire types to include "unknown" change_status Backend ChangeStatus now emits "unknown" via _unresolvable_cte_change fallback (recce/util/breaking.py:277-280) and dbt_adapter propagation. Frontend wire-format types were narrower than what the backend serializes, so values are stripped on the type boundary and papered over with `as` casts downstream. Addresses Andy's review on 37b2b43: - info.ts:10 NodeColumnData.change_status (Finding #2) - info.ts:81 MergedNodeData.change_status (sibling of #3 at merged-node level) - info.ts:84 MergedNodeData.change.columns (Finding #1) - cll.ts:37 CllNodeData.change_status (Finding #3) - patchLineageDiffFromCll.ts:28,30 column-change locals (consequence of widening #2) Branch C reachability (Notes) is left as documented dead-but-defensive per the test pin in 37b2b43 — handled as a separate follow-up. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * chore: scrub customer + partner names from test docstring Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * chore: restore Paradime context in test docstring Paradime is a platform partner, not a customer — naming it in the test docstring is fine and adds technically meaningful context. The over-aggressive scrub from d7d238a removed it; restoring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * fix(cll): coerce wire "unknown" change_status to "modified" at renderer boundary Round 2 of Andy's review on PR #1375 (sha c7b031e → NO-GO): the wire-type widening in ac544cd (`info.ts:10,81,84`, `cll.ts:37`) was correct but only covered one layer. The next internal layer (`ColumnAnnotation.changeStatus`, `LineageGraphColumnNode.data.changeStatus`, `LineageGraphNode.data.changeStatus`) is still narrow `"added" | "removed" | "modified"`, and the `as`-casts at `computeColumnLineage.ts:44,68`, `lineage.ts:147`, and `utils.ts:124` silently carried `"unknown"` through. The CLL renderer (`LineageColumnNode.tsx:171,193`, `LineageCanvas.tsx:159`, `LineageLegend.tsx:116-119`) indexes by `ChangeStatus` and returns `undefined` for `"unknown"` — no tinted bg, no left accent, empty 14×14 indicator. Visually identical to "no change" — DRC-3409's exact symptom, one layer deeper on the CLL graph surface. Fix: coerce `"unknown"` → `"modified"` at the CLL data-flow boundary via a single `coerceCllChangeStatus` helper. Rationale: - Backend `_diff_select_scope` fallback (`breaking.py:277-282`) emits `"unknown"` when it knows an upstream CTE changed but cannot attribute the change to a specific outer column. The user intent is "definitely a change, unattributable" — exactly the behavior the modified `~` amber indicator conveys. - Coercion at the boundary keeps the wire format honest (analytics/logging preserve the "unknown" signal upstream of the renderer) while internal graph types stay narrow and the existing palette/symbol tables stay unchanged — no UX call required for a new color/symbol. - Matches Andy's suggested fix path ("upstream needs to coerce 'unknown' → 'modified' before reaching the CLL renderer"). Touched call sites: - `computeColumnLineage.ts:44,68` (walk + start-seed annotation writes) - `lineage.ts:147` (LineageGraphColumnNode column write in pre-CLL render path) - `utils.ts:124` (buildLineageGraph node-level change_status write) New unit coverage: 5 tests on the helper itself plus integration through `computeColumnLineage` (both walk and start-seed paths), all green at 3761 total frontend tests. Frontend lint + type-check + tests all pass at this commit. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> * chore: remove PR-verification screenshots from branch PR is approved by reviewer; screenshots served their purpose. Removing per team repo-hygiene policy — repo shouldn't carry PR-verification artifacts. The Screenshot Evidence table in the PR description still documents what each capture verified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: even-wei <evenwei@infuseai.io> --------- Signed-off-by: even-wei <evenwei@infuseai.io> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Wei-Chun (Andy), Chang <wcchang@infuseai.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first version of the server