feat(data-modeling): concise freshness preview + cross-DAG duplication report - #69578
Conversation
…n report The preview command dumped every node's cadence, flooding the ArgoCD terminal past the scrollback. Default to a compact per-DAG summary (tier counts + plan counts + warnings); move the per-node walls behind --verbose. Add a team-level cross-DAG duplication report so a redundant DAG (double-materializing shared saved queries) can be judged safe to drop only when it holds no queries unique to it.
|
Reviews (1): Last reviewed commit: "feat(data-modeling): concise freshness p..." | Re-trigger Greptile |
| def _print_cross_dag_duplication(self, team_id: int) -> None: | ||
| """Which saved queries a team materializes in more than one DAG, and which are unique to | ||
| each DAG. A query in >1 DAG is materialized redundantly; a DAG whose queries all also live | ||
| elsewhere is safe to drop, one with unique queries is not.""" | ||
| dags = list(DAG.objects.filter(team_id=team_id)) | ||
| if len(dags) < 2: | ||
| return |
There was a problem hiding this comment.
Redundant DB query for team DAGs
_print_cross_dag_duplication always re-fetches all team DAGs with a fresh DAG.objects.filter(team_id=team_id), even when handle() already loaded them at line 42 (the common no---dag-id case). The handle() result can't be passed directly because --dag-id scopes it to one DAG, but a simple fix is to pass all_dags as a parameter that handle() fetches separately (before applying any --dag-id filter) — eliminating the duplicate query without changing behaviour.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Read-only management command change (no schedules created/updated/deleted) by an owning-team author with STRONG familiarity, with new/updated tests covering both the verbose gating and the new duplication report; the sole unresolved review comment is a minor redundant-query perf nit, not a correctness or safety issue.
- Author wrote 100% of the modified lines and has 11 merged PRs in these paths (familiarity STRONG).
- greptile-apps[bot] reviewed the current head.
- Unresolved Greptile P2 comment suggesting avoiding a redundant DB query in _print_cross_dag_duplication — a performance nit, not a correctness/safety concern, so non-blocking but worth addressing in a follow-up.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 104L, 1F substantive, 154L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (154L, 2F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 33da132 · reviewed head 1447da4 |
Problem
preview_freshness_schedulesprinted every node's effective cadence for each DAG. On a real customer DAG that's 150+ lines, which scrolls out of the ArgoCD terminal buffer before you can read the tier plan or the warnings. And when a team runs several DAGs, there was no way to see that they redundantly materialize the same saved queries in more than one DAG, which is the thing you need to know before dropping a legacyDefaultDAG.Changes
tiers: 15min x23 30min x85 1day x67), a one-line plan (plan: CREATE 4, UPDATE 0, DELETE 0), and the warning lines (or⚠ none). The per-node cadence walls and tier membership move behind--verbose, so nothing is lost for anyone who wants the detail.safe to drop, one with unique queriesunsafe to drop. This is the safeguard for consolidating a team down to a singleposthog_$teamIDDAG without silently losing a query that lived only in the DAG being removed.Still strictly read-only: no schedule is created, updated, or deleted, and no live read/write path is touched.
Example on a team with two overlapping DAGs:
How did you test this code?
pytest products/data_modeling/backend/test/test_preview_freshness_schedules.py(5 passed). Updated the two existing tests for the new summary format, and added two cases: one asserting the per-node detail is verbose-only (the flood regression), one asserting the cross-DAG report counts double-materialized queries and flags the unsafe-to-drop DAG. I (Claude) did not run this against a live cluster; the dedup-by-saved_query_idkey was validated against production metadata (172/295 shared for the example team).🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built by Claude (Claude Code) after the author hit the terminal-flood problem running the deployed preview command against real teams, and raised the DAG-consolidation question (many v2 teams carry a redundant
DefaultDAG alongsideposthog_$teamID). Decisions: summary-by-default with--verboseopt-out (chosen over keeping verbose default) so the common case is readable; dedup onsaved_query_idrather than node name, confirmed correct via the Node model'sunique(team, dag, saved_query)constraint (a saved query appears once per DAG but can span DAGs) and against prod metadata. The duplication safeguard lives in preview rather than a separate command so it is the single place to judge a drop. Kept out of the write-path PR (#67554) since it is read-only tooling.