wu-sheng opened a new pull request, #54:
URL: https://github.com/apache/skywalking-horizon-ui/pull/54

   ## Why
   
   In multi-entity compare, the merged **"All"** tab of a top-N widget inferred 
its asc/des direction from a single probe entity's values. That heuristic flips 
when the probe entity's values are flat/equal — e.g. **"Top 10 instances by 
success rate"** where one service's instances are all at `100%` — or when no 
entity has ≥2 rows (it defaults to descending).
   
   Result: a `top_n(service_instance_sla,10,asc)` widget — deliberately 
**worst-first** for triage (lowest success ≈ highest error) — rendered 
**best-first** in the "All" list, while the per-entity tabs (native MQE order) 
stayed correct. (Two screenshots in the discussion: per-entity "App" tab 
ascending 42.9→52.6, but "All" descending 100→42.9.)
   
   ## What
   
   Resolve the direction **BFF-side, at template-resolve time** — where MQE is 
understood — instead of guessing in the UI from data:
   
   - `DashboardWidget.topNOrder?: 'asc' | 'des'` (api-client) — the resolved 
direction.
   - `widgetsForScope` parses the first `top_n(…, asc|des)` of each 
`top`/`record` widget onto `topNOrder` (pass-through + reference-stable for 
everything else, so no per-call allocation when nothing needs it).
   - The UI's `multiTopGroups` reads `topNOrder` (default `des`) and **drops 
the data-probe** — the UI no longer parses MQE or infers direction from one 
entity's (possibly flat) values.
   
   This kills both failure modes (single-probe fragility + desc-on-ambiguity) 
at the source and keeps the UI MQE-agnostic. Top-N order is a plain literal arg 
today, so the parse is trivial and lives in the one place that understands MQE.
   
   ## Validation
   
   - `pnpm -r run type-check` ✓ · UI + BFF `build` ✓ · `pnpm -r run lint` 0 ✓ · 
`pnpm -r run test:unit` — **BFF 116 / UI 116** ✓ (adds `topNOrderOf` parser 
tests: asc/des/missing-arg/non-top_n/empty, plus a `widgetsForScope` enrichment 
test) · `license:check` 0 invalid ✓.
   - **Live OAP** (demo.skywalking.apache.org): the config route now serves 
`topNOrder` resolved from the stored template — 
`top_apis`/`top_instance_load`/`top_instance_slow`/`slow_statements` = `des`, 
**`top_instance_sla` = `asc`** (the bug case). So the "All" merge now ranks 
worst-first for success rate.
   - No new UI strings (i18n unaffected). Follow-up to #53 (fixes a bug in that 
still-unreleased preview feature).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to