feat(dashboards): add Streamlit agent analytics dashboard - #480
feat(dashboards): add Streamlit agent analytics dashboard#480whuang602 wants to merge 5 commits into
Conversation
caohy1988
left a comment
caohy1988
left a comment
There was a problem hiding this comment.
P0: 1 / P1: 3 / P2: 1 — REQUEST_CHANGES
Reviewed HEAD 08d42ac55e3dd8641d16502f0c1aaa24b01296e0. The PR is still OPEN + DRAFT. The acknowledged anti-drift work remains a readiness requirement; this is not LGTM pending only an undraft.
-
P0 — Report billed bytes accurately.
dashboards/streamlit/queries.py:790returnsjob.total_bytes_processed, whichdashboards/streamlit/app.py:575-580totals and labels billed. These are different BigQuery statistics. A synthetic successful job with 1,024 bytes processed and 10,485,760 bytes billed renders “1.0 KB billed”. This silently understates the dashboard's FinOps accounting. Carrytotal_bytes_billedseparately for on-demand billing, or label this measure “processed” and remove the billing claim. Preserve zero incremental usage for cache hits. The BigQuery job contract distinguishes the fields and qualifies billed bytes under capacity pricing. -
P1 — Preserve selected filters when options expire or fall outside the cap.
dashboards/streamlit/app.py:90-91,192-194removes selections absent from refreshed options;dashboards/streamlit/models.py:242then turns an empty selection into___ALL___. Streamlit AppTest reproduces selected agentalpha/ sessions1becoming all agents / all sessions when those options disappear, without an explicit filter-clear action. This can happen after narrowing the window or after newer sessions displace the selected session from the 1,000-option list. Preserve the selected values in the widget's option set so queries retain their scope and return no matching data when appropriate. -
P1 — Keep truncated IDs reachable, as Grafana does.
dashboards/streamlit/app.py:197-201,213-217uses multiselects withoutaccept_new_optionsor another exact-ID input, whiledashboards/streamlit/queries.py:146-148caps each option list at 1,000. Streamlit 1.49 defaults custom values to disabled. In a busy window, an older known session/user cannot be selected even by typing its exact ID; the shortest available window can still contain more than 1,000 IDs.grafana/README.md:192-194explicitly guarantees custom values for Agent, User ID and Session. Add the equivalent parameterized input path and retain those values across option refreshes. -
P1 — Land the promised Streamlit contract tests and CI before readiness.
dashboards/streamlit/queries.py:82-113,639-669,726-838introduces filter, cap and cache contracts, but this nine-file diff contains no tests or anti-drift checker. The existing.github/workflows/grafana-sync-check.yml:20-31watches Grafana paths only, and the current checker verifies Grafana SQL/JSON, not these Python builders. Land the acknowledged draft follow-up with explicit panel/filter mappings, whole-session rollups, missing-data behavior, bound parameters, real-job byte caps, cache-scope cases and filter-retention regressions; ensure Streamlit changes trigger it. -
P2 — Document typed-view setup before the run command.
dashboards/streamlit/README.md:17-42goes directly from environment configuration to launch. An existing raw events table alone is insufficient for the overview latency, LLM and tool panels. Include theviews create-allprerequisite with matching project/dataset/table and view-prefix guidance, plus the query identity's jobUser/dataViewer requirements. The missing-view error handler already provides a recovery command, but setup should establish this dependency first.
Validation: all 15 builders parsed as single SELECT/UNION queries; identifier-injection rejection, bound filter parameters, cache misses for changed filter/project/dataset/window/cap, repeated-query cache hits, and over-cap refusal passed in offline probes. Each real-job configuration retained maximum_bytes_billed. A synthetic four-tab AppTest passed on Streamlit 1.49.0 / Plotly 6.0.0. Existing Grafana sync + typed-view tests: 37 passed; the standalone checker verified 37 Grafana queries. Wheel metadata and a .[dashboards] dependency-resolution dry run passed. No customer BigQuery queries were executed; no source fixes were made.
Pairing with #481: links resolve at this HEAD. When Grafana moves, reconcile dashboards/README.md:10 and the old grafana/queries/README.md references in app.py:210 / queries.py:86, along with the new anti-drift CI paths. This is merge-order coordination, not an additional current-path defect.
caohy1988
left a comment
caohy1988
left a comment
There was a problem hiding this comment.
REVIEW_VERDICT=REQUEST_CHANGES
P0=0 / P1=2 / P2=1
Astra re-review of exact HEAD 11b2624560615d3b6dedc0c2097e9155e5b7ef8e, including the complete delta from 08d42ac55e3dd8641d16502f0c1aaa24b01296e0. Counts are deduplicated by root cause: the two prior filter findings now share one demonstrated widget-state defect. The PR remains OPEN + DRAFT.
| Prior finding | Disposition at this HEAD | Evidence |
|---|---|---|
| P0: processed bytes labeled billed | CLOSED | Actual fetch() -> scan_log -> footer path reports 10.0 MB billed (1.0 KB processed) for a synthetic job with those distinct statistics. Repeated local-cache reads add zero bytes; a BigQuery cache hit reports zero billed bytes. |
| P1: expired selections silently become ALL | PARTIAL; blocker remains | Explicit pruning is removed and selected values are added to options, but real Streamlit 1.49.0 widgets still reset their selections when the options change. See R1. |
| P1: capped-out exact IDs unreachable | PARTIAL; blocker remains | Agent/User/Session now enable custom input, but entering a new ID and clicking Apply resets it to ALL. See R1. |
| P1: missing Streamlit contract tests/CI | OPEN | No tests, checker, or workflow changes in the four-file follow-up. See R2. |
| P2: typed-view setup before launch | PARTIAL | Setup order, query IAM roles, and the create-all command are added; the custom-prefix flag is invalid. See R3. |
R1 — P1: Preserve filter values across widget identity changes. dashboards/streamlit/app.py:221-252, particularly the _merge_widget_options() calls. On the declared minimum streamlit==1.49.0, options participate in widget identity even with a key. Appending the active values keeps them available but does not restore the selection when Streamlit registers the replacement widget.
Actual AppTest reproduction: select agent alpha, user u1, event LLM_RESPONSE, and session s1; submit; refresh options so those values disappear. All four returned selections and the resulting BigQuery array parameters become ['___ALL___'], with no explicit clear. The same reset occurs when an unselected option is merely added, or the list is reordered. Separately, enter custom-agent, custom-user, and custom-session and click Apply: merging these newly entered values changes the widget identities immediately, and all three submitted filters become ALL. These probes exercise the actual sidebar and query-parameter builder, not just the helper in isolation. Preserve submitted selections independently of widget recreation and test this through Apply and subsequent refreshes on the supported minimum version. The 1.49 custom-input contract permits new values without inserting them into the options list.
R2 — P1: Land the promised Streamlit contract coverage and run it in CI. There are still no Streamlit tests or anti-drift checks in this PR. .github/workflows/grafana-sync-check.yml:20-31 only watches Grafana/checker paths; general CI installs .[dev], which does not include the Streamlit extra. Add the acknowledged panel/filter mappings, whole-session and missing-data cases, bound parameters, real-job byte caps, billing/cache accounting, and widget retention/custom-ID regressions, with Streamlit dependencies and triggers. The reproduced R1 failure is exactly what the current passing Grafana tests do not exercise.
R3 — P2: Use the supported custom-prefix CLI flag. dashboards/streamlit/README.md:58 names --view-prefix, but views create-all exposes --prefix (src/bigquery_agent_analytics/cli.py:1848). CLI parsing of the documented flag exits 2 with “No such option”; the same invocation with --prefix custom_ reaches the mocked view manager and exits 0. Change the flag and keep BQ_VIEW_PREFIX aligned with it.
Delta assessment: no independent additional P0/P1 found beyond these unresolved contracts. The newly enabled custom-input path's immediate silent-ALL behavior is included in R1, not counted again. The billing fix is credited as closed.
Pairing with #481: it is still OPEN at 47620b8f05926568ba4f94968a248f6935f3fd79; Grafana has not moved in #480. Current paths resolve. Combining the branches has a verified .gitignore three-way conflict. Whichever lands second must preserve both integrations' ignore rules and reconcile the legacy-path protection identified in #481. After the move, update dashboards/README.md:10 to grafana/README.md, plus app.py:243, queries.py:86, and models.py:17 to the new repository paths; target the new locations in anti-drift coverage. This remains merge-order coordination, not an extra current-path finding.
Validation: 106 existing Grafana-sync/local-runner/typed-view tests passed; standalone checker verified 37 Grafana queries. All 15 Streamlit builders parsed as single read-only queries; synthetic cap/cache/billing probes passed; four-tab AppTest smoke passed with Streamlit 1.49.0 / Plotly 6.0.0. Four refresh scenarios and custom-ID submission reproduce R1. No live BigQuery jobs or source implementation changes. No undraft or merge; Haiyuan remains the gate.
|
@whuang602 Astra re-review comments for HEAD REVIEW_VERDICT=REQUEST_CHANGES Astra re-review of exact HEAD
R1 — P1: Preserve filter values across widget identity changes. Actual AppTest reproduction: select agent R2 — P1: Land the promised Streamlit contract coverage and run it in CI. There are still no Streamlit tests or anti-drift checks in this PR. R3 — P2: Use the supported custom-prefix CLI flag. Delta assessment: no independent additional P0/P1 found beyond these unresolved contracts. The newly enabled custom-input path's immediate silent-ALL behavior is included in R1, not counted again. The billing fix is credited as closed. Pairing with #481: it is still OPEN at Validation: 106 existing Grafana-sync/local-runner/typed-view tests passed; standalone checker verified 37 Grafana queries. All 15 Streamlit builders parsed as single read-only queries; synthetic cap/cache/billing probes passed; four-tab AppTest smoke passed with Streamlit 1.49.0 / Plotly 6.0.0. Four refresh scenarios and custom-ID submission reproduce R1. No live BigQuery jobs or source implementation changes. No undraft or merge; Haiyuan remains the gate. |
| with st.sidebar.form("filters"): | ||
| agents = st.multiselect( | ||
| "Agent", | ||
| options=_merge_widget_options("flt_agent", options.get("agent", [])), |
There was a problem hiding this comment.
P1 — R1: Preserve filter values across widget identity changes
_merge_widget_options() keeps selected values in the options list, but on Streamlit 1.49.0 options still participate in widget identity. When options change (refresh/reorder/new custom IDs), the multiselect recreates and submitted selections become ___ALL___.
Reproduced (AppTest): select agent alpha / user u1 / event LLM_RESPONSE / session s1 → refresh so those values leave the option set → all four filters and BigQuery array params become ['___ALL___']. Same reset when entering custom IDs and clicking Apply.
Preserve submitted selections independently of widget recreation; test through Apply + refreshes on the supported minimum. 1.49 custom-input allows new values without inserting them into options.
Shared root cause for prior “expired → ALL” and “custom ID → ALL” findings.
| --table-id YOUR_TABLE | ||
| ``` | ||
|
|
||
| > **Note:** Views default to the `adk_` prefix. If you specify a custom prefix with `--view-prefix`, set `BQ_VIEW_PREFIX` in `.env` (or in the dashboard sidebar) to match. |
There was a problem hiding this comment.
P2 — R3: wrong CLI flag for custom prefix
Docs name --view-prefix, but views create-all exposes --prefix (cli.py). --view-prefix exits 2 (“No such option”); --prefix custom_ works. Please fix the documented flag and keep BQ_VIEW_PREFIX aligned.
| "pandas>=2.0.0", | ||
| "python-dotenv>=1.0.0", | ||
| ] | ||
| streamlit = [ |
There was a problem hiding this comment.
P1 — R2: Streamlit contract tests + CI still missing
No Streamlit tests/checker/workflow landed in this follow-up. Existing Grafana sync CI only watches grafana/** + the Grafana checker (not in this PR's path set), and general CI installs .[dev] without the Streamlit extra.
Please add the acknowledged contracts (panel/filter mappings, whole-session + missing-data, bound params, byte caps, billing/cache, widget retention / custom-ID regressions for R1) and wire CI so Streamlit path changes run them with the Streamlit deps.
Rebase note after #481 merge (PE)
Conflict (only file)
Recommended resolution: take Also ensure these post-#481 lines from dashboards/grafana/datasource.yaml
dashboards/grafana/.local/
dashboards/grafana/*.jsonPath leftovers still on this branch (from prior Astra review)After resolving
CI path filters in Suggested author stepsgit fetch upstream main
git rebase upstream/main # or merge
# resolve .gitignore as above
# fix the three Streamlit/README path strings
git push --force-with-lease # if rebasingHappy to re-review once the conflict is cleared. |
Python-native alternative to the Grafana dashboard, backed by the same typed views and filter semantics. Includes scan-cap guardrails and query caching. Intentionally leaving existing Streamlit content intact.
…parity - Resolve billed vs processed bytes disclaimers - Preserve active filter selections and enable exact-ID entry for truncated options - Document typed-view prerequisites and IAM roles before launch instructions - Add query anti-drift sync checker and comprehensive contract test suites
…gleCloudPlatform#481 alignment, and CI check and tests
11b2624 to
9c71f08
Compare
|
Summary of status:
|
caohy1988
left a comment
caohy1988
left a comment
There was a problem hiding this comment.
REVIEW_VERDICT: CHANGES
P0=0 / P1=2 / P2=2
Re-reviewed exact HEAD 9c71f08d0923e06ddb834e81f2e05494a76da545, including the follow-up from 11b2624 and the full Streamlit addition against rebased main. REQUEST_CHANGES.
| Prior finding | Disposition | Evidence at this HEAD |
|---|---|---|
| R1 P1: filter values lost across widget identity changes | PARTIAL | Applied filters now survive option retirement, addition, reordering, and an empty refresh, including subsequent Apply. First custom-ID Apply still submits ALL; changing/clearing an existing selection on the next Apply is also ignored. Details below. |
| R2 P1: missing Streamlit contract tests/CI | PARTIAL | A dedicated workflow, 14-query SQL sync checker, mutation checks and dashboard unit tests are present. Runtime widget/cache contracts are mocked away, including the reproduced R1 failures. |
R3 P2: README --view-prefix |
CLOSED | dashboards/streamlit/README.md:58 now uses --prefix, matching views_create_all in src/bigquery_agent_analytics/cli.py:1848. |
| Prior billing P0: processed bytes labeled billed | CLOSED | Successful-job path reports 10.0 MB billed (1.0 KB processed) from distinct statistics; repeated local-cache reads add zero. The legacy tuple fallbacks are removed. R5 below is a separate exception-path defect. |
R1 — P1 (confidence 10/10): Preserve the newly submitted values before widget recreation. dashboards/streamlit/app.py:233–267,296–303. The new default=_default_for(applied.agents) and equivalent arguments use the previous applied filters, while _seed_options() adds pending custom values before widget registration. On supported Streamlit 1.49.0, both options and defaults participate in identity.
Real AppTest, production sidebar and BigQuery parameter builder:
- From ALL, enter
custom-agent,custom-user,custom-sessionand click Apply once: all three submitted arrays are['___ALL___']; the custom values are in the option lists but unselected. - Apply
alpha/u1/LLM_RESPONSE/s1, then change to existingbeta/u2/TOOL_END/s2and click Apply again: all four arrays retain the first values. Clearing them on that next Apply likewise retains the first values. The changed defaults recreate the widgets before the pending edit is committed.
The applied-state anchor fixes refresh retention, but it must also preserve the current form submission before re-registration can overwrite it. Regress custom first-submit, successive edits and clearing through actual widget-to-query behavior. The 1.49 custom-input contract returns new values without adding them to the option menu.
R2 — P1 (confidence 10/10): Exercise real Streamlit behavior in the new CI tests. tests/test_dashboards_streamlit_app.py:37–44,153–160,1153–1195. Streamlit is unconditionally replaced with a mock, cache decorators become passthrough functions, and the sidebar test replaces multiselect with return state.get(key, []). Installing Streamlit in the workflow therefore does not exercise widget identity, reruns, or actual cache keys. As an independent mutation check, removing all four default= repair arguments in memory still passes all five filter-retention/custom-ID tests. Add real AppTest regressions on the supported minimum, asserting bound arrays after Apply/refresh; retain real cache-key variation coverage. Credit the new SQL mapping/checker and CI wiring as delivered.
R4 — new P2 (confidence 10/10): Keep the inspected trace selected when recent sessions refresh. dashboards/streamlit/app.py:605: chosen = st.selectbox("Session", options=ids, index=0). Real AppTest of row_sessions: select s2 from ['s1','s2']; refresh to ['s3','s1','s2']; the picker and actual bound session_ids silently switch to ['s3'] although s2 remains available. A newly arriving session can switch the trace being investigated. Preserve the chosen session across option changes while it remains eligible, and make any necessary reset explicit. This is the separate trace selector, not the shared sidebar filters.
R5 — new P2 (confidence 10/10): Retain billing statistics after result-download failure. dashboards/streamlit/queries.py:824–830 returns bytes_processed=0, bytes_billed=0 for every exception, including failure inside job.to_dataframe() after successful query execution. An offline probe using an actual SDK QueryJob with synthetic DONE/no-error metadata (1,024 processed; 10,485,760 billed), and an injected row-download ServiceUnavailable, produces “1 queries this run · 0 B billed (0 B processed)” through fetch → scan_log → footer. Preserve known completed-job statistics on materialization failure; show unknown when statistics are unavailable. The real-job cap remains set. R4/R5 are newly identified defects in the original addition, not claims that the latest commits introduced them.
CI evidence: the six green check results are five GitHub Actions security-scan jobs plus CLA. At this exact SHA, general CI and Streamlit query sync check both report action_required; neither establishes passing application CI.
Validation: 142 targeted dashboard/Streamlit-sync/Grafana-sync/local-runner tests passed locally; standalone checkers verified 14 Streamlit and 37 Grafana queries. All 15 Streamlit builders parsed as single read-only queries. Bound-parameter, real-job cap, cache-scope and successful billing probes passed; four-tab AppTest smoke passed (8 metrics). Pyink 25.12.0, isort and diff whitespace checks passed for the relevant changed files. Runtime probes used Streamlit 1.49.0 / Plotly 6.0.0 / BigQuery SDK 3.45.0. Synthetic data only; no live BigQuery jobs, source fixes, merge or undraft. #481 is now in the base; the migrated Grafana links/checker paths resolve.
caohy1988
left a comment
caohy1988
left a comment
There was a problem hiding this comment.
REVIEW_VERDICT: CHANGES
P0=0 / P1=2 / P2=2
Independent review of exact HEAD 9c71f08d0923e06ddb834e81f2e05494a76da545. I did not take prior review claims at face value: I reproduced every runtime finding below from scratch with real Streamlit AppTest (no mocks of streamlit) in throwaway venvs at both the pinned minimum 1.49.0 and latest 1.63.0, plus unit-level probes with synthetic BigQuery job metadata. Where my repro diverges from the prior review, I say so.
P1-1 — Filter Apply is broken on the declared minimum Streamlit 1.49.0 (dashboards/streamlit/app.py:234-303, pyproject.toml:63).
AppTest driving the real sidebar_filters on streamlit 1.49.0:
- From ALL, type custom IDs (
custom-agent/custom-user/custom-sessionviaaccept_new_options) and click Apply once → all three bind('___ALL___',); the custom values appear in the option lists but are not submitted. - Apply
alpha/u1/LLM_RESPONSE/s1(binds correctly), then change tobeta/u2/TOOL_END/s2and Apply again → all four arrays still bind the first values. - Mechanism:
_seed_optionsmerges the in-progress value intooptions, anddefault=_default_for(applied)changes with each apply; on 1.49.0 both participate in widget identity, so the submit rerun recreates the widget and resets it to the previous applied values before the pending edit is read. - The same script on streamlit 1.63.0 passes every step, so this is version-dependent — but
streamlit>=1.49.0makes 1.49.0 a supported configuration. Either restructure the seeding/default so options and default are stable across the submit rerun, or raise the version floor to a release where this works. - One divergence from the prior review, for honesty: in my repro, clearing an existing selection and applying DID bind the
___ALL___sentinel on 1.49.0. I could not reproduce the "clearing is ignored" sub-claim; the custom-ID and successive-edit failures reproduced exactly. - What works on both versions: retention of applied filters across an empty/failed options refresh (widget and
applied_filtersboth keep the selection).
P1-2 — The new CI tests never exercise real Streamlit, and no application CI has run on this SHA (tests/test_dashboards_streamlit_app.py:37-44, .github/workflows/streamlit-sync-check.yml).
streamlitis in_UNCONDITIONAL_MOCKS: even though the workflow installs.[dev,streamlit], the suite replaces Streamlit with a MagicMock, turnscache_data/cache_resourceinto passthroughs, and swapsmultiselectforreturn state.get(key, [])— a fake that ignoresdefault=entirely. The P1-1 defects are interactions betweendefault,options, and widget identity, so they are structurally invisible to this suite: the tests assert the app's own merging logic, not widget behavior. Verified locally: the full 62-test suite passes on a machine without streamlit installed at all.- Add real AppTest regressions (minimum supported version) that assert the bound query-parameter arrays after Apply/refresh, and at least one test that exercises real
st.cache_datakey variation. - CI state verified via the Actions API at this SHA: the
CIworkflow and the newStreamlit query sync checkworkflow both reportaction_required— the only green checks are the zizmor security scans and CLA. So no test or sync-check signal exists for this HEAD yet. - Credit where due: the 14-query SQL sync checker is real and passes locally (
All 14 Streamlit dashboard queries match canonical Grafana SQL), and the SQL-sync CI wiring itself is sound.
P2-1 — Session selectbox refresh silently switches the trace under investigation (dashboards/streamlit/app.py:605).
Real AppTest of row_sessions (reproduced on both 1.49.0 and 1.63.0, so not version-dependent): select s2 from [s1, s2] → trace query binds ('s2',); options refresh to ['s3', 's1', 's2'] (a new session arrives) → the picker and the bound session_ids silently switch to ('s3',) even though s2 is still eligible. Preserve the chosen session while it remains in the options; reset explicitly only when it leaves.
P2-2 — Billing statistics are dropped on result-download failure (dashboards/streamlit/queries.py:771-786,824-830).
Probe with a synthetic completed job (total_bytes_processed=1024, total_bytes_billed=10,485,760, cache_hit=False) whose to_dataframe() raises: run_query returns bytes_processed=0, bytes_billed=0, so fetch → scan_log → footer renders "1 queries this run · 0 B billed (0 B processed)" for a query that actually billed 10 MB — contradicting the footer's "what this rerun actually scanned" contract. On the materialization-failure path the job object and its statistics exist; report them (or say "unknown") instead of zero.
Verified as solid (no action needed):
- Injection posture: table/view identifiers are regex-validated before interpolation, filter values go out as
ArrayQueryParameters,LIMITs are int-cast, timestamp literals are built from owneddatetimes. - Success-path billing now reports distinct billed vs processed stats correctly (10.0 MB billed / 1.0 KB processed for a synthetic job), and the run-id dedup keeps cached reruns from double-counting.
- Dry-run preflight +
maximum_bytes_billedcap is set on the real job; guardrail message path works. - 62 mock-based tests and the sync checker pass locally; README/
.env.sample/.gitignore(SA JSON keys) are consistent.
Verdict: REQUEST_CHANGES — two P1s (broken Apply on the supported minimum Streamlit; mock-only widget/cache test coverage with no CI signal on this SHA) and two P2s (trace-selectbox hijack on refresh; billing stats lost on download failure), each reproduced independently against real Streamlit.
…eal AppTest CI coverage
caohy1988
left a comment
caohy1988
left a comment
There was a problem hiding this comment.
Fresh review of exact head 295542a64e59eafe7a76b3a251388ff14057f6f3.
Requesting changes for one P1 compatibility regression and one P2 cost-reporting issue. I also left one non-blocking P3 CI-quality comment.
Prior-review disposition:
- The active custom-value, successive-apply, and clear-selection cases now pass on current Streamlit.
- R3 (
--prefix) and R4 (session selector retention) are fixed. - The known-statistics half of R5 is fixed: a completed job whose stats are available preserves them after result-download failure.
- R1 is not fully closed at the declared Streamlit floor, and R5 still conflates unavailable statistics with zero.
Verification:
- Python 3.13 / Streamlit 1.63.0: all 102 focused tests pass.
- Python 3.13 / Streamlit 1.49.0 (the declared minimum): 101 pass, 1 fails. The failing committed AppTest is
test_filter_widget_options_change_preserves_applied_filters. - Manual AppTest continuation on 1.49.0: after option refresh, the widget value becomes empty while
applied_filtersstill containsagent-a; the next Apply changes it to___ALL___. - All 14 Streamlit queries pass the canonical Grafana SQL anti-drift checker.
compileall,git diff --check, andisort --check-onlypass.pyink --checkreportsdashboards/streamlit/charts.pywould be reformatted (missing final newline).- The PR's CI and Streamlit workflows are currently
action_requiredfor this fork, so there is no application CI result on this head.
I did not run a live BigQuery integration query in this pass.
| "python-dotenv>=1.0.0", | ||
| ] | ||
| streamlit = [ | ||
| "streamlit>=1.49.0", |
There was a problem hiding this comment.
P1: the declared Streamlit minimum silently broadens refreshed filters
This lower bound is not compatible with the state-retention implementation at this head. With Python 3.13 and Streamlit 1.49.0, the branch's own test_filter_widget_options_change_preserves_applied_filters fails: after agent-a disappears from the refreshed BigQuery options, the code re-adds it to the option list, but the widget value is [], not ['agent-a']. Continuing the real AppTest and clicking Apply changes applied_filters.agents from ('agent-a',) to ('___ALL___',), so every panel is silently broadened.
The same 102-test set passes on Streamlit 1.63.0, so current CI resolution masks a broken advertised floor. Please either make the widget/session-state pattern work on 1.49.0 or raise the minimum to the first version you verify, and add a minimum-dependency CI cell so this contract stays exercised.
| bytes_billed = int(exc.bytes_billed) | ||
| cache_hit = bool(exc.cache_hit) | ||
| else: | ||
| bytes_processed, bytes_billed, cache_hit = 0, 0, False |
There was a problem hiding this comment.
P2: unavailable billing statistics are still presented as zero
This fallback makes an unknown outcome indistinguishable from a proven zero. A query can be accepted and billed before to_dataframe() or the metadata reload fails; this path then records (0, 0, False), fetch() appends it to the scan log, and the footer describes the total as what the rerun actually billed/processed.
There is a second instance in _extract_job_stats: if total_bytes_processed is populated but total_bytes_billed is None, it does not reload and returns billed bytes as zero. An exact-head probe produced (2048, 0, False) with no reload call.
Please preserve unknown explicitly (optional fields or a stats_known flag), attempt hydration whenever a required statistic is missing, and have the footer identify failed queries with unavailable cost data rather than summing zero.
| run: pip install -e ".[dev,streamlit]" | ||
|
|
||
| - name: Validate Streamlit queries match canonical Grafana SQL | ||
| run: python3 scripts/check_streamlit_queries_sync.py |
There was a problem hiding this comment.
P3 (non-blocking): dashboard Python bypasses the formatting gate
This workflow runs the SQL checker and tests, but no formatter. The repository's autoformat.sh and the main CI dirty-diff assertion cover only src/, tests/, examples/, and scripts/, so the new production Python under dashboards/streamlit/ is outside both paths. The gap is observable on this head: pyink --check dashboards/streamlit/charts.py wants to add the missing final newline.
Please add dashboards/streamlit to the shared format command and CI diff list, or run pyink --check and isort --check-only for it here.
2 participants
What: Python-native alternative to the Grafana dashboard, backed by the same typed views and filter semantics, essentially extending existing Grafana features into Streamlit. Includes scan-cap guardrails and query caching. Intentionally leaving existing Streamlit content intact.
Why: Addresses the optional dashboard integrations mentioned in #339. This is intentionally paired with PR #481, which is working to consolidate 3P dashboard integration into a dedicated dashboards/ directory.
Reason this is in draft: Anti-drift check script & tests are in review.