fix(dashboard): upgrade UI Kit to v2.28.0 to clamp traffic curve at baseline (#1113)#1123
fix(dashboard): upgrade UI Kit to v2.28.0 to clamp traffic curve at baseline (#1113)#1123AustinChangLinksys wants to merge 1 commit into
Conversation
…aseline (#1113) Bump ui_kit_library and generative_ui from v2.27.0 to v2.28.0. v2.28.0 adds AppLineChart.preventCurveOverShooting (default true) which constrains cubic-bezier smoothing so the curved, filled line cannot dip below the lowest data point. This fixes the Traffic Monitor card whose upload/download curve could overshoot below the y=0 baseline in near-zero troughs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 1 · 69eb8f1..51a45b3 (full)
Verdict: 💬 Self-review (comment only) — Austin's own PR; GitHub prohibits self-approve. Full review for reference.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| High | pubspec.yaml:62,66 |
[both reviewers] pubspec.lock not in PR diff — builds may resolve against stale SHA |
|
| High | pubspec.yaml:59-67 |
Version sync risk: ui_kit_library and generative_ui have no atomic bump guard |
|
| High | lib/page/dashboard/views/components/usp_traffic_analysis_card.dart:~650 |
Pre-existing: _DualAxisLinePainter bypasses ui_kit_library — baseline fix won't reach Trends dual-axis chart |
|
| Med | test/golden_test/page/dashboard/ (absent) |
No golden/screenshot regression test for the visual curve-clamping fix | |
| 💡 | Med | pubspec.yaml:62 |
PR description lacks link to ui_kit v2.28.0 release notes for auditability |
Confidence: High = code-verified · Med = located + reasoned, not fully confirmed · Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.
⚠️ Warning Details
W-1 — pubspec.lock not updated in PR diff — High [both reviewers]
pubspec.yaml:62 / pubspec.yaml:66
The diff updates both ui_kit_library and generative_ui from v2.27.0 to v2.28.0 in pubspec.yaml, but no pubspec.lock changes appear in the PR. Flutter's Git-sourced dependencies require the lock to be committed alongside pubspec.yaml so all environments resolve to the same resolved-ref commit SHA. Local lock file currently references v2.26.1 / ae0f9848.... If CI caches or relies on the old lock, built binaries may not include the v2.28.0 baseline-clamp fix.
Fix: Run flutter pub get on the PR branch and commit the updated pubspec.lock.
W-2 — Dual sub-package version sync has no atomic guard — High [single reviewer B]
pubspec.yaml:59-67
Both ui_kit_library and generative_ui point to the same Git repo (privacyGUI-UI-kit.git) and must stay on the same ref, but are managed as two separate YAML stanzas. A future bump that edits only one stanza is syntactically valid but semantically broken (API mismatch).
Fix: Add an inline comment # NB: keep ui_kit_library and generative_ui on the same ref above both entries, or a scripts/bump_ui_kit.sh helper that atomically updates both lines.
W-3 — Pre-existing: _DualAxisLinePainter bypasses ui_kit — ui_kit baseline fix may not reach Trends chart — High [single reviewer B] (pre-existing, not introduced by this PR)
lib/page/dashboard/views/components/usp_traffic_analysis_card.dart:~650
The Trends dual-axis chart uses _DualAxisLinePainter extends CustomPainter, drawing directly on Flutter Canvas. AppLineChart/AppBarChart/AppPieChart from ui_kit are used elsewhere in the same file (lines ~229, ~349, ~414) but the dual-axis chart is custom. If the #1113 overshoot also manifests in the Trends view, the v2.28.0 upgrade will not fix it there.
Fix: Verify whether #1113 overshoot occurs in the Trends view. If yes, a separate fix to _DualAxisLinePainter's baseline clamping is needed.
W-4 — No golden/screenshot regression test for visual fix — Med [single reviewer B]
test/golden_test/page/dashboard/ (no traffic-card golden test found)
This PR resolves a visual rendering defect (curve below baseline) but no golden test for UspTrafficAnalysisCard exists. The project already uses alchemist in dev_dependencies.
Fix: Add a golden test verifying the traffic curve does not render below the baseline.
✅ What looks good
- Version bump is minimal and clean: two lines in one file.
- Both
ui_kit_libraryandgenerative_uiare synchronised onv2.28.0in this PR (correct). - No hardcoded secrets, no application logic changes, no injection vectors.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 2 · 69eb8f1..51a45b3 (full)
Verdict: 💬 Self-review (comment only) — Austin's own PR; GitHub prohibits self-approve. Full review for reference.
Note: Prior Round 1 review at same SHA already on record. This round adds new findings from deeper inspection.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| 🟢High | pubspec.lock:639-640,1466-1467 |
[both reviewers] Lock pins both packages to v2.26.1 (SHA ae0f984) — two versions behind; v2.28.0 fix not actually locked in | |
| 🟢High | pubspec.lock:643 vs 1470 |
[both reviewers] generative_ui lock reports version:"1.0.0" while ui_kit_library is "2.26.1" at same SHA — version scheme incoherent |
|
| 🟢High | pubspec.yaml:61-62,65-66 |
[both reviewers] Mutable git tags with no SHA pin; without updated lock, tag force-push is undetectable | |
| 🟢High | pubspec.yaml:62,66 |
[both reviewers, prior] pubspec.lock absent from PR diff — stale from prior round; still unresolved |
|
| 🟢High | pubspec.yaml:59-67 |
Version sync risk: no atomic bump guard for the two co-versioned packages (no script, no CI rule) — still unresolved | |
| 🟢High | lib/page/dashboard/views/components/usp_traffic_analysis_card.dart:~650 |
[both reviewers, prior, pre-existing] _DualAxisLinePainter bypasses ui_kit_library; v2.28.0 baseline clamp does not reach Trends dual-axis chart |
|
| 🟡Med | test/golden_test/page/dashboard/ (absent) |
No golden/screenshot regression test for visual fix — still unresolved |
Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.
⚠️ Warning Details
N-1 — pubspec.lock pins both packages to v2.26.1, NOT v2.28.0 — 🟢High [both reviewers] BLOCKS MERGE
pubspec.lock:639-640 / pubspec.lock:1466-1467
# pubspec.lock — generative_ui (confirmed on disk)
generative_ui:
description:
ref: "v2.26.1" ← stale tag
resolved-ref: ae0f98480fd19184c43acc306fadc1ab03f42963
url: "https://github.com/linksys/privacyGUI-UI-kit.git"
version: "1.0.0"
# pubspec.lock — ui_kit_library (confirmed on disk)
ui_kit_library:
description:
ref: "v2.26.1" ← stale tag
resolved-ref: ae0f98480fd19184c43acc306fadc1ab03f42963
version: "2.26.1"pubspec.yaml declares ref: v2.28.0 for both packages, but the committed pubspec.lock still resolves to SHA ae0f98480fd19184c43acc306fadc1ab03f42963 (v2.26.1). Flutter Git dependencies are pinned by the resolved-ref SHA in the lock file. This means:
flutter pub get --offline→ gets v2.26.1 (baseline clamp fix absent)flutter pub get --frozen(CI hardened mode) → build rejected (lock contradicts yaml)- Non-frozen CI silently pulls v2.28.0 without that SHA being in the reviewed diff
The advertised fix (#1113, traffic curve clamping) is not reproducibly present in the repository as committed. The lock was already stale before this PR (v2.26.1 lock vs. v2.27.0 declared ref) — this PR would make the gap two full versions.
Fix: Run flutter pub get on the PR branch and commit the updated pubspec.lock.
N-2 — generative_ui version string "1.0.0" diverges from ui_kit_library "2.26.1" at same SHA — 🟢High [both reviewers]
pubspec.lock:643 vs 1470
Both packages resolve to identical commit ae0f98480fd19184c43acc306fadc1ab03f42963, yet their Dart version: fields differ. The generative_ui/pubspec.yaml inside the monorepo declares 1.0.0 independently of the parent's tag. Consequences: flutter pub outdated reports generative_ui as perennially behind; any future automated version-keying logic will silently skip it; the two packages' API surfaces cannot be compared by version number alone.
Fix: Align generative_ui's version: field with the monorepo tag scheme (or document the intentional independent versioning).
W-2 (prior, still open) — No atomic bump guard — 🟢High [single reviewer B]
The scripts/ directory does not exist. No bump_ui_kit.sh, Makefile target, or CI rule enforces joint update of the two ref: lines. A partial bump (one updated, one forgotten) is syntactically valid but semantically broken.
Fix: Add # NB: keep ui_kit_library and generative_ui on the same ref inline comment above both stanzas, plus a scripts/bump_ui_kit.sh or CI lint rule.
W-3 (prior, pre-existing) — _DualAxisLinePainter bypasses ui_kit — 🟢High [both reviewers]
lib/page/dashboard/views/components/usp_traffic_analysis_card.dart:~650-788
// Line 757 (Reviewer A confirmed on disk):
final ratio = yMax > 0 ? (values[i] / yMax).clamp(0.0, 1.0) : 0.0;This is a self-contained CustomPainter drawing directly to Flutter Canvas. The v2.28.0 clamp fix applies only to AppLineChart/AppBarChart from ui_kit. The Trends dual-axis chart will still exhibit the pre-fix rendering after this PR.
Fix: Verify if #1113 overshoot manifests in the Trends view. If yes, a separate fix to _DualAxisLinePainter's baseline is needed.
✅ What looks good
- Version bump is minimal and clean: two lines in one file.
- Both
ui_kit_libraryandgenerative_uiare synchronised onv2.28.0in this PR (correct). - No hardcoded secrets, no application logic changes, no injection vectors.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
Summary
Fixes #1113 — the Traffic Monitor card's upload/download curve could overshoot below the y=0 baseline (out of border) in near-zero troughs.
Upgrades
ui_kit_libraryandgenerative_uifrom v2.27.0 → v2.28.0. UI Kit v2.28.0 addsAppLineChart.preventCurveOverShooting(defaulttrue), which constrains the cubic-bezier smoothing of curved, filled lines so the line and its filled area cannot dip below the lowest data point / y=0 baseline.Related UI Kit issue
The root-cause fix lives in the UI Kit library — see linksys/privacyGUI-UI-kit#3 (AppLineChart curved line dips below the y=0 baseline in near-zero troughs), resolved in v2.28.0 (commit
49a0b044).Why no app-side code change
preventCurveOverShootingdefaults totrue, so the fix takes effect automatically after the version bump. The Traffic Monitor card (usp_traffic_analysis_card.dart) usesAppLineChartwithfilled: true, which is exactly the case the UI Kit issue describes.Changes
pubspec.yaml: bumpui_kit_libraryrefv2.27.0→v2.28.0pubspec.yaml: bumpgenerative_uirefv2.27.0→v2.28.0UI Kit v2.28.0 changelog reference
Testing
flutter pub getresolvesui_kit_libraryto v2.28.0flutter analyzeon chart-using files: no new errorsCloses #1113