[PR] fix(ai): stop grading a group healthy from an unresolved consumer lag [rocketmq-dashboard]

["unbridled-41 (via GitHub)" <gi...@apache.org>]

unbridled-41 opened a new pull request, #5091:
URL: https://github.com/apache/rocketmq-dashboard/pull/5091

   ## Problem
   
   `rmq.group.detail` grades a consumer group's health from three inputs — consume-stat availability, connection availability and the total lag. The ladder enumerates the first two "signal unavailable" cases but then compares the lag directly:
   
   ```java
   } else if (group.getOnlineInstances() == 0) {
       status = "WARNING";
       reasons.add("The group has no online consumer.");
   } else if (group.getTotalLag() > 0) {
       status = "WARNING";
       reasons.add("The group has accumulated messages.");
   } else {
       status = "HEALTHY";
   }
   ```
   
   `-1` is the lag-unknown sentinel (`ConsumerLagResolver.UNKNOWN`), so it is a *successful* comparison against `> 0` and the group is graded **HEALTHY** — a definite "nothing has accumulated" verdict derived from data the server marks undeterminable.
   
   ## Evidence
   
   The state is reachable with `consumeStatsAvailable == true`: `RocketMQMetadataProvider` sets it as soon as `examineConsumeStats` returns (`:347`) and then writes the sentinel when any queue diff is the broker's own `-1` for 5.0 gRPC consumers (`:368`, `lagUnknown ? ConsumerLagResolver.UNKNOWN : totalLag`). The same vocabulary is already used on the read side: `GroupDetailOutput.Progress.from` collapses a queue lag of `-1` into `totalLag = -1`, and the contract documents `-1` for `onlineInstances` as "the connection inventory is unavailable".
   
   Reproduction (added test, mock only): `ConsumerGroupVO{consumeStatsAvailable=true, onlineInstances=1, totalLag=-1}` → before the fix `health = {"status":"HEALTHY","reasons":[]}`; expected `UNKNOWN` with a reason.
   
   Existing coverage only pins the two other unavailable signals: `detailMarksHealthUnknownWhenConsumerConnectionsAreUnavailableTest` (`onlineInstances=-1`) and `detailWithoutOnlineConsumersKeepsProgressAndClientsEmpty` (`consumeStatsAvailable=false`). There was no `totalLag == -1` case.
   
   ## Root cause
   
   The verdict ladder was written when the total lag was always a measurement. The later "preserve unknown lag" work introduced the `-1` sentinel across the provider, the aggregate VO and the tool's own progress block, but the health ladder was not updated, so the sentinel falls through to the healthy branch.
   
   ## Fix
   
   Test the sentinel before the lag-derived branches and answer `UNKNOWN` with `"Consumer lag information is unavailable."`, mirroring the two existing unavailable-signal branches. The branch is placed after the connection-derived ones on purpose: `onlineInstances == 0` is a definite fact independent of the lag, so that group keeps its `WARNING` rather than degrading to `UNKNOWN`. Both boundaries are pinned by tests.
   
   ## Priority
   
   **60** = impact 18 (a wrong positive verdict is exactly what an operator or an agent acts on during triage; no data corruption) + blast radius 10 (one tool and its frozen MCP/JSON contract, used by the playground, the hosted agent and external MCP clients) + reproducibility 18 (deterministic, mock-only, no broker needed) + maintenance 14 (the ladder has to track the sentinel vocabulary the rest of the contract already uses).
   
   `FIX_CONFIDENCE` **88** — three lines plus tests; the only behaviour change is the group that was mis-graded.
   
   ## Tests
   
   Two new tests: `detailMarksHealthUnknownWhenTheConsumerLagIsUnavailableTest` **fails before the fix** (`expected: "UNKNOWN"`, actual `HEALTHY`), and `detailKeepsTheConnectionWarningWhenTheConsumerLagIsUnavailableTest` pins the boundary that stays `WARNING`.
   
   ```
   $ mvn -o test -Dtest=ConsumerGroupReadToolHandlersTest
   Tests run: 6, Failures: 0, Errors: 0, Skipped: 0
   BUILD SUCCESS
   
   $ mvn -o test -Dtest='org.apache.rocketmq.studio.ops.ai.tool.**'
   Tests run: 166, Failures: 0, Errors: 0, Skipped: 0
   BUILD SUCCESS
   (checkstyle:validate ran as part of both builds)
   ```
   
   ## Risk
   
   Low. The contract's `health.status` enum already includes `UNKNOWN`, so no schema change is needed; only a group whose lag is genuinely unresolved can change status, and it moves from an unjustified `HEALTHY` to an explicit `UNKNOWN` with a reason.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(ai): stop grading a group healthy from an unresolved consumer lag [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins merged PR #5091:
URL: https://github.com/apache/rocketmq-dashboard/pull/5091


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org