Repository navigation
fix(minimax-bot): sort the job rows before the fold takes them, clarify the README - #52
Conversation
The fold of the job queue kept the rows in the order the jobs arrived in, and only used the status to decide which three rows stay in the open. A queued job could therefore sit above a job that was already working, and the strip read as if the queue had not started. The rows are sorted before the fold takes the first three now: the jobs that are already working come first, the ones waiting for another job next, the queued ones last. The sort is stable, so jobs of the same kind keep the order they arrived in. A strip that fits in the open rows has nothing to fold and keeps the order the official build draws, so the rows of a quiet chat are the official ones.
The Background work paragraph said the strip folds "when more than three are queued". The fold counts every job the strip lists, the ones that are already working included, and it keeps the first three of them in the open. The sentence now says that.
hetaoBackend
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. The change is small and correct: the comparator is safe (renderHelpers already filters to the three statuses in HELPER_STATUS_ORDER), the version is bumped consistently in plugin.json, package.json and mcp.mjs, and the new assertions fail against the current main app.js and pass with this change. One design question and two nits inline, none blocking.
| const keep = new Set(ranked.slice(0, HELPER_ROWS_PREVIEW)); | ||
| return items.filter((item) => keep.has(item)); | ||
| function orderedHelperRows(items, foldable) { | ||
| if (!foldable) return items; |
There was a problem hiding this comment.
The order now depends on how many jobs are listed, and that causes two things:
- With three or fewer jobs, a queued job can still sit above a running one, which is the situation this PR describes as the bug. The new assertion
plain[0] = Queued task 1in the test locks that in as expected behaviour. - When the fourth job arrives the whole strip reorders, and when an open queue drops back to three the rows return to arrival order. The same rows visibly swap places at the threshold.
With three or fewer jobs, sorting does not change which rows are shown, only their order, and the PR treats the sorted order as the better one. Could we always sort (and drop the foldable parameter)? If matching the official order below the threshold is a hard requirement, a comment explaining what it buys would help the next reader.
| return items.filter((item) => keep.has(item)); | ||
| function orderedHelperRows(items, foldable) { | ||
| if (!foldable) return items; | ||
| return [...items].sort((a, b) => HELPER_STATUS_ORDER[a.status] - HELPER_STATUS_ORDER[b.status]); |
There was a problem hiding this comment.
Nit: the commit message and PR description say jobs of the same kind "keep the order they arrived in", but the input here is [...state.helpers, ...state.tasks] (line 1254), so the pre-sort order is helpers first, then tasks, not chronological. The stable sort keeps order within helpers and within tasks, but a queued helper created later still sorts ahead of an earlier queued task. The test shows this too: Answer the ticket starts after every helper. Either add a tie-breaker such as (a.startedAt ?? a.createdAt) - (b.startedAt ?? b.createdAt), or reword the claim.
| - **Group chats**: the group admin answers by default. `@Name` calls on a specific member, and a Bot that `@`-mentions another member in its reply hands the turn over. Members with nothing to add reply `[PASS]` and are skipped. Each member keeps a separate session per group, and groups cannot see private chats. There is no fixed reply cap. Discussions finish naturally, or wait for more instructions after repeated handoffs without tool progress. | ||
| - **Hand-offs between Bots**: `message_bot` hands work to another Bot and immediately returns an admission receipt; the completed result returns to the source chat. `post_group` is **private chat only**: it posts to a group and returns immediately while the discussion continues according to the group's reply mode. From a private chat, Chief can post to any group without joining it; other Bots can only post to groups they belong to. During a group turn, `post_group` is disabled for everyone, including Chief and admins, for both same-group and cross-group posts. Use `@Name` in a reply to hand off within the group; start cross-group work from a private chat. | ||
| - **Background work**: when an isolated execution context is useful, a Bot calls `start_helper` to run it in the background in a fresh, separate session; the Bot's turn ends at once and you can keep chatting. The Bot simply says something like "let me look into it" without internal jargon or IDs. When the job finishes, its report comes back to the Bot as a new turn, and the Bot reviews it and passes it on in its own words (in groups too, where it can keep handing off with `@`). At most 2 background jobs run at once (one slot is always kept free for chat) and the rest queue; each Bot can have at most 4 queued or running. Running jobs are shown above the chat input and can be stopped at any time; when more than three are queued, the jobs that are already working stay in the open and the rest fold into one line that says how many are left, and one button below the rows opens the whole queue; tool approvals from a background job appear in its chat; jobs still running when the Mini App restarts are marked interrupted. Bots can also manage jobs with `list_helpers` / `stop_helper`. "Allow background helpers automatically" is on by default in Bot settings; when off, a Bot only does so when you explicitly ask. | ||
| - **Background work**: when an isolated execution context is useful, a Bot calls `start_helper` to run it in the background in a fresh, separate session; the Bot's turn ends at once and you can keep chatting. The Bot simply says something like "let me look into it" without internal jargon or IDs. When the job finishes, its report comes back to the Bot as a new turn, and the Bot reviews it and passes it on in its own words (in groups too, where it can keep handing off with `@`). At most 2 background jobs run at once (one slot is always kept free for chat) and the rest queue; each Bot can have at most 4 queued or running. Running jobs are shown above the chat input and can be stopped at any time; when more than three jobs are listed, the strip sorts them by how far along they are, the ones already working first and the ones waiting for another job next, keeps those first three in the open and folds the rest into one line that says how many are left, and one button below the rows opens the whole queue; tool approvals from a background job appear in its chat; jobs still running when the Mini App restarts are marked interrupted. Bots can also manage jobs with `list_helpers` / `stop_helper`. "Allow background helpers automatically" is on by default in Bot settings; when off, a Bot only does so when you explicitly ask. |
There was a problem hiding this comment.
Nit on wording: in "…keeps those first three in the open…" it is unclear what those refers to; "keeps the first three in the open" reads more clearly. The sentence also gives working and waiting jobs a position but not queued ones, so "and queued ones last" would complete the order.
MyPrototypeWhat
left a comment
There was a problem hiding this comment.
Thanks for the quick follow-up! This is a clean fix for #51.
- The sort is correct:
Array.prototype.sortis stable, so jobs with the same status stay in their original order, and running → waiting_for_tasks → queued come first before the top 3 are taken. - When there are ≤3 rows,
orderedHelperRowsreturnsitemsuntouched, so the original order is kept. The new test checks this. - An empty list returns early. With exactly 3 rows the strip isn't foldable, so nothing is reordered and no fold button shows.
- The version is 0.7.7 in all three places:
package.json,.minimax-plugin/plugin.json, andserverInfoinmcp.mjs. - The README wording now matches the code: the fold counts every listed job, running ones included.
- The change only touches minimax-bot.
Locally npm run check passes (Node 22), and the plugin's node --test tests passes 205/205 (Node 20).
One small, non-blocking suggestion (inline): HELPER_STATUS_ORDER[status] is undefined for a status outside the map, so the comparator would return NaN and the order would be undefined. Today renderHelpers filters items down to the three known statuses, so this can't happen yet. A fallback rank such as ?? 99 would keep it safe if a new status is added later.
| return items.filter((item) => keep.has(item)); | ||
| function orderedHelperRows(items, foldable) { | ||
| if (!foldable) return items; | ||
| return [...items].sort((a, b) => HELPER_STATUS_ORDER[a.status] - HELPER_STATUS_ORDER[b.status]); |
There was a problem hiding this comment.
Nit (non-blocking): a status missing from HELPER_STATUS_ORDER gives undefined, so the comparator returns NaN and the order becomes undefined. The filter in renderHelpers only lets the three known statuses through today, but a fallback would keep this safe later:
| return [...items].sort((a, b) => HELPER_STATUS_ORDER[a.status] - HELPER_STATUS_ORDER[b.status]); | |
| const rank = (item) => HELPER_STATUS_ORDER[item.status] ?? Number.MAX_SAFE_INTEGER; | |
| return [...items].sort((a, b) => rank(a) - rank(b)); |
The rows of the strip were sorted only while the fold was in play, so a queued job could sit above a job that was already working when three jobs or fewer were listed, and the rows swapped places when the fourth one arrived. The strip sorts them now, always. The status comes first: the jobs already working, then the ones waiting for another job, then the queued ones. Jobs of the same kind are ordered by the time they arrived, which is the moment they started or the moment they were queued. A status the strip does not know yet comes last, so a new one cannot scramble the order.
The Background work paragraph described the fold but not the order the rows are in. It now says that the strip orders the jobs by how far along they are and keeps the first three in the open.
|
Thanks, both. All four comments are addressed in the two commits on top. Always sort (hetaoBackend). The strip sorts the rows in every case now, and the Arrival time as the tie-breaker (hetaoBackend). Jobs of the same kind are ordered by the time they arrived, the moment they started or the moment they were queued. The claim in the commit message and in the description now holds on its own, and the pre-sort order, helpers first and then tasks, no longer decides it. README wording (hetaoBackend). "keeps the first three in the open" instead of "those first three", and the order now names the queued ones last. Fallback rank (MyPrototypeWhat). Taken as written: The test changed with it: the assertion that a queued job stays on top below the threshold is gone, the strip of two jobs now asserts that the running job comes first, and a new case checks that two queued jobs keep the order they arrived in. |
MyPrototypeWhat
left a comment
There was a problem hiding this comment.
Thanks for the quick update! The new commits look good to me.
helperRank()falls back toNumber.MAX_SAFE_INTEGER, so a status the strip doesn't know yet sorts last instead of giving aNaNcomparison.- The arrival tie-break (
startedAt ?? createdAt ?? 0) is safe. Both fields are numericDate.now()values everywhere they are set,??also coversnull, and0is a sensible floor when both are missing. Within the same status the order is now deterministic: oldest first. - Always sorting, even when ≤3 rows are listed, makes the strip consistent, and the README now describes exactly that: working, then waiting, then queued, with the first three kept in the open.
- The version is still 0.7.7 in
package.json,plugin.json, andserverInfo, and the change stays within minimax-bot.
Locally npm run check passes (Node 22), and the plugin's node --test tests passes 205/205 (Node 20).
This is the follow-up to #51, which merged the fold of the background job queue and the flat reply bubbles. Two things were still open after the merge, and both are fixed here:
What changed
Test environment and results
npm run checkin the repository root reports no errors and no warnings, and the 69 contract tests pass.Unverified
这是 #51 的后续。#51 合并了后台任务队列的折叠和回复气泡的平铺。合并之后还开着两件事,这两件都在这里修掉:
改了什么
测试环境与结果
npm run check无错误无警告,69 条契约用例通过。未验证