feat(ui): group child sessions by role - #249
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (9)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (6)📚 Learning: 2026-08-04T02:36:32.766ZApplied to files:
📚 Learning: 2026-07-31T19:15:09.621ZApplied to files:
📚 Learning: 2026-08-10T05:11:10.183ZApplied to files:
📚 Learning: 2026-08-02T22:13:53.026ZApplied to files:
📚 Learning: 2026-08-02T22:13:53.026ZApplied to files:
📚 Learning: 2026-07-21T16:54:32.671ZApplied to files:
🔇 Additional comments (10)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe session picker now categorizes descendant sessions into synthetic groups under root main sessions. The grouped tree preserves ordering and state, tracks task totals, handles orphaned sessions, and returns roots with flattened nodes. Regression coverage validates categorization and edge cases. ChangesSession picker grouping
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SessionData
participant SessionNormalization
participant TreeConstruction
participant SessionPicker
SessionData->>SessionNormalization: normalize sessions and resolve root ancestors
SessionNormalization->>TreeConstruction: provide categories and group metadata
TreeConstruction->>SessionPicker: return roots and flattened nodes
SessionPicker->>SessionPicker: refresh sorting and render stored task totals
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Criterion
Details
| Benchmark suite | Current: f6bc162 | Previous: 9276e1e | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6674787 ns/iter (± 126990) |
6655811 ns/iter (± 84472) |
1.00 |
fib/jit_watchdog |
2226991 ns/iter (± 10257) |
2752794 ns/iter (± 4592) |
0.81 |
fib/jit_none |
2220904 ns/iter (± 29959) |
2710554 ns/iter (± 57051) |
0.82 |
fib/interp_mlua_hook |
8112705 ns/iter (± 96049) |
8029305 ns/iter (± 19218) |
1.01 |
fib/interp_watchdog |
4253491 ns/iter (± 14618) |
4330743 ns/iter (± 24960) |
0.98 |
fib/interp_none |
4281112 ns/iter (± 27626) |
4319876 ns/iter (± 12009) |
0.99 |
buffer_rw/jit_mlua_hook |
584785 ns/iter (± 3872) |
585211 ns/iter (± 1120) |
1.00 |
buffer_rw/jit_watchdog |
191802 ns/iter (± 414) |
192038 ns/iter (± 778) |
1.00 |
buffer_rw/jit_none |
191451 ns/iter (± 229) |
191390 ns/iter (± 246) |
1.00 |
buffer_rw/interp_mlua_hook |
1044777 ns/iter (± 13300) |
1043308 ns/iter (± 15953) |
1.00 |
buffer_rw/interp_watchdog |
585293 ns/iter (± 4464) |
581669 ns/iter (± 2217) |
1.01 |
buffer_rw/interp_none |
584853 ns/iter (± 1556) |
581999 ns/iter (± 3602) |
1.00 |
splash_render_120x40 |
70904 ns/iter (± 6267) |
67289 ns/iter (± 6460) |
1.05 |
splash_render_200x60 |
175867 ns/iter (± 15788) |
149308 ns/iter (± 28066) |
1.18 |
This comment was automatically generated by workflow using github-action-benchmark.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5db320870e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| plugins/sessions/init.lua | Core change: replaces the old parent-child tree traversal with a two-phase flat-grouping approach. Descendants are bucketed by role under each main root. Minor concerns around unescaped Lua pattern injection in has_word and a nil-arithmetic assumption in synthetic_group; both are safe given the current call flow. |
| n00n-lua/tests/plugin_host.rs | Adds a thorough integration test covering category assignment, orphan handling, agent bucketing (>20 items), total_tasks accuracy, and stale group_id clearing. Node-count assertion of 36 is arithmetically correct given the 29-session fixture, 5 synthetic groups, and 2 overflow buckets. |
| changelog.d/session-category-groups.changed.md | New changelog fragment summarising the UI change. No code impact. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[refresh: all sessions] --> B[assign_ranks]
B --> C[build_tree]
C --> D{kind == main\nand no parent_id?}
D -- yes --> E[Add to roots\nInit descendants list]
D -- no --> F[main_ancestor walk]
F --> G{Found main root?}
G -- yes --> H[Add to descendants of root]
G -- no --> I[Promote to orphan root]
H --> J[descendant_category]
J --> K{kind == team / workflow?}
K -- yes --> L[teams / workflows bucket]
K -- no --> M{Title matches\nresearch/ponder/draft/review?}
M -- yes --> N[Named category bucket]
M -- no --> O{kind == task / agent?}
O -- yes --> P[agents bucket]
O -- no --> Q[other bucket]
L & N & P & Q --> R[synthetic_group node\ntotal_tasks = child count]
R --> S[root.children list]
E & I --> T[table.sort roots by_recency]
S --> T
T --> U[collect → flat nodes list]
U --> V[group_node: bucket categories\n> MAX_GROUP_CHILDREN into sub-buckets]
V --> W[table.sort board.nodes]
W --> X[apply_filter / render]
Prompt To Fix All With AI
### Issue 1
plugins/sessions/init.lua:168-174
**Unescaped pattern injection in `has_word`**
`word` is concatenated directly into Lua pattern strings. All current `NAMED_CATEGORIES` values (`"research"`, `"ponder"`, `"draft"`, `"review"`) are purely alphanumeric, so this is safe today. However, if any future entry contains a Lua pattern special character (`.`, `+`, `*`, `(`, `)`, `%`, `[`, `]`, `^`, or `$`), it will silently produce incorrect matches or a runtime error. A one-time escape would future-proof all four patterns.
### Issue 2
plugins/sessions/init.lua:226-228
**Nil-arithmetic crash if child rank is absent**
`rank[children[1].id] - 0.25` will throw `attempt to perform arithmetic on a nil value` if the session has no entry in `rank`. In the production path, `assign_ranks(all)` is always called before `build_tree`, so every session has a rank and this never fires. Still, there is no guard, so a missed rank (e.g., a future call site or test that skips `assign_ranks`) would produce an opaque crash rather than a graceful fallback.
### Issue 3
plugins/sessions/init.lua:184
**Redundant field inclusion in category-match text**
`normalize_session` sets `display_title = title` whenever `display_title` is absent, so for the majority of sessions `title` and `display_title` are identical. Concatenating both means every keyword present in the title appears twice in `text`, which is harmless for correctness but makes the matching logic harder to reason about. Using `title` alone — or `display_title` alone, since it is always the canonical label after normalization — would be simpler and equally correct.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(ui): group child sessions by role" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@n00n-lua/tests/plugin_host.rs`:
- Line 2741: Replace the #[test] annotation on this test with the required
#[test_case] annotation, preserving the test body and existing behavior.
In `@plugins/sessions/init.lua`:
- Around line 168-194: Update has_word to recognize any non-alphanumeric
character as a word boundary, including punctuation such as periods, question
marks, and parentheses. Preserve exact matches and existing category detection
behavior in descendant_category.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad5bbb44-e016-45c7-97f9-e1418d4b1cce
📒 Files selected for processing (3)
changelog.d/session-category-groups.changed.mdn00n-lua/tests/plugin_host.rsplugins/sessions/init.lua
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Test (Windows)
- GitHub Check: Lint (macOS)
- GitHub Check: Docs
- GitHub Check: Build
- GitHub Check: Build (Windows)
- GitHub Check: Coverage
- GitHub Check: Test (macOS)
- GitHub Check: Lint (Windows)
- GitHub Check: Rustdoc
- GitHub Check: Test
- GitHub Check: Unused deps
- GitHub Check: MSRV (1.97)
- GitHub Check: Lint
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Criterion
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Do not add unsafe code, FFI, global mutable state,static mut, or unchecked transmute-like behavior without written review, an explicit lint exception, and a SAFETY comment where applicable.
Do not useunwrap,expect,panic!,todo!,unimplemented!, ordbg!in production Rust code; tests are exempt from the unwrap/expect/panic restriction.
Do not silently discard failures withunwrap_or,unwrap_or_default,.ok()onResult, or equivalent defaults; return typed errors, reject the operation, or use an explicitly named fallback with sanitized structured logging.
Use idiomatic Rust, descriptive names, minimal state, and avoid unnecessary comments, bloat, and magic numbers or strings.
Import types at the top of the file and use short imported names; keep constants immediately after imports.
UseResult<T, E>and explicit error handling instead of panics; usethiserrorfor library/domain errors andcolor-eyreat binary edges.
Use#[derive(Copy)]only for structs containing one primitive field.
Prefer structured logging with useful fields and provide helpful, sanitized error messages.
Place unit tests in the same file inside#[cfg(test)]modules; use#[test_case]and snake_case test names.
Propagate typed errors with?,ok_or_else, andmap_err; library crates usethiserrorand binaries usecolor-eyre.
Treat LLM and provider output as untrusted input; validate schemas, domain constraints, and source evidence before persistence or action.
Do not log raw provider payloads, prompts, credentials, or user session data, and never commit credentials, API keys, tokens, cookies, or auth headers.
Validate and authorize HTTP, file, queue, configuration/environment, LLM, and provider-callback inputs before mutation or persistence.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Files:
n00n-lua/tests/plugin_host.rs
🧠 Learnings (1)
📚 Learning: 2026-07-31T19:15:04.814Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 206
File: changelog.d/orchestration-hardening.fixed.md:1-1
Timestamp: 2026-07-31T19:15:04.814Z
Learning: Files in changelog.d are changelog fragments intended for user-facing release notes and may begin directly with summary prose. Do not flag a missing Markdown H1 or require an H1 solely because Markdownlint MD041 reports it in these fragment files.
Applied to files:
changelog.d/session-category-groups.changed.md
🪛 markdownlint-cli2 (0.23.1)
changelog.d/session-category-groups.changed.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (4)
plugins/sessions/init.lua (2)
36-46: LGTM!Also applies to: 163-166, 196-207, 488-488
209-227: 🎯 Functional CorrectnessVerify rank order for bucketed categories.
If
by_recencyuses ascending rank for equal timestamps, Line 227 ranks a category behind the first bucket frommake_bucket. Both nodes usechildren[1].updated_at. Line 609 can then place the bucket before its category inboard.nodes.Inspect
by_recencyandapply_filter. If filtering or navigation consumes this order, reserve the category rank before its bucket and add a regression with more thanMAX_GROUP_CHILDRENdescendants.#!/bin/bash set -euo pipefail rg -n -P -C 16 \ '^\s*local function (by_recency|apply_filter)\b|^\s*rank\[[^]]+\]\s*=|^\s*table\.sort\(board\.nodes' \ plugins/sessions/init.luaAlso applies to: 605-609
n00n-lua/tests/plugin_host.rs (1)
2742-2836: LGTM!changelog.d/session-category-groups.changed.md (1)
1-1: LGTM!
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
All three Codex issues have been addressed in the latest push:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
The |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
|
@coderabbitai please re-review; the |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Changed files ---'
git diff --name-only HEAD^ HEAD
printf '%s\n' '--- Lua structure ---'
ast-grep outline plugins/sessions/init.lua --items all --type function --match 'synthetic_group|has_word|build|sort' || true
printf '%s\n' '--- Current rank and category-related code ---'
rg -n -C 8 'synthetic_group|rank\[category_id\]|has_word|category_id|children\[1\]' plugins/sessions/init.lua
printf '%s\n' '--- Current Rust test annotations and test body ---'
rg -n -C 5 'sessions_picker_groups_descendants_by_category_without_dropping_orphans|#\[test(_case)?' n00n-lua/tests/plugin_host.rs
printf '%s\n' '--- PR patch for relevant files ---'
git diff HEAD^ HEAD -- plugins/sessions/init.lua n00n-lua/tests/plugin_host.rsLength of output: 99532 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="w0wl0lxd/n00n"
pr="249"
printf '%s\n' '--- Pull request metadata ---'
gh api "repos/${repo}/pulls/${pr}" \
--jq '{number, state, head: .head.sha, base: .base.sha, updated_at, html_url}'
printf '%s\n' '--- Pull request files ---'
gh api --paginate "repos/${repo}/pulls/${pr}/files?per_page=100" \
--jq '.[] | {filename, status, additions, deletions, sha}'
printf '%s\n' '--- Pull request commits ---'
gh api "repos/${repo}/pulls/${pr}/commits?per_page=100" \
--jq '.[] | {sha, message: .commit.message}'Length of output: 2831
I will re-review PR
|
|
Re: test attribute on line 2743 - The test |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Criterion'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: 63757d8 | Previous: ab3f257 | Ratio |
|---|---|---|---|
buffer_rw/jit_watchdog |
192393 ns/iter (± 966) |
80574 ns/iter (± 5571) |
2.39 |
buffer_rw/jit_none |
191808 ns/iter (± 717) |
89730 ns/iter (± 3476) |
2.14 |
This comment was automatically generated by workflow using github-action-benchmark.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review @greptile-apps please review @coderabbitai review |
|
|
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Validation
RUST_TEST_THREADS=1 cargo test -p n00n-lua --test plugin_host sessions_cargo clippy -p n00n-lua --tests -- -D warningscargo nextest run -p n00n-luacargo fmt --all -- --checkstylua --check plugins/sessions/init.luagit diff --check