atiaomar1978-hub commented on PR #25411: URL: https://github.com/apache/camel/pull/25411#issuecomment-5229737511
## Bugbot & Grok Review Report _AI-generated review on behalf of the operator._ Full automated review was performed on the changeset before merge. **All must-fix items were addressed** in commit `d5170133bf4`. --- ### Verdict Implementation matches [CAMEL-24374](https://issues.apache.org/jira/browse/CAMEL-24374). Feature shape is correct: **Ctrl+G** opens a tree-style node navigator; **`g`** remains for route-only goto. Test coverage is solid for the core paths (16 unit tests, all green). --- ## Bugbot Findings ### Must-fix (all fixed in `d5170133`) | # | Severity | Finding | Fix | |---|----------|---------|-----| | 1 | **High** | Processor-level `id:` lines overwrote `currentRouteId`, mis-attributing later nodes and breaking filter grouping | Route ids are only read **before** the route header is emitted; processor ids are ignored | | 2 | **High** | New `- from:` block did not reset `currentRouteId`, leaking the previous route's id | Reset only on top-level `- from:` (not nested `from:` under `- route:`) | | 3 | **Medium** | After a route, top-level YAML (`- beans:`, REST verbs, etc.) was indexed as processors | Route scope ends when dedented past the active route indent | | 4 | **Medium** | `routeKey` used `filePath + routeId`, collapsing duplicate route ids in one file during filter | Grouping key is now `filePath + routeFromLine` (unique per route instance) | --- ## Grok Findings ### Must-fix (all fixed in `d5170133`) | # | Finding | Fix | |---|---------|-----| | 1 | Processor `id:` overwrote route id and polluted labels (`myLog` instead of `hello`) | Scoped route id reads; labels prefer `message`/`constant` over `id` | | 2 | Route id leaked across subsequent `- from:` routes | Clear `currentRouteId` only on top-level `- from:` | | 3 | Filter `routeKey` collapsed distinct routes sharing an id | `routeFromLine` propagated to all entries | | 4 | Ctrl+G cross-file goto could discard unsaved edits via `loadFile()` | `openFileAt()` returns early when `sourceViewer.isEditMode() && sourceViewer.isDirty()` | ### Should-fix (partially addressed / deferred) | # | Finding | Status | |---|---------|--------| | 5 | Tree is flat (always `indent=1`) | **Partially fixed** — indent now scales with YAML depth; full EIP nesting (choice/when) deferred | | 6 | Block `to`/`toD` showed as type `uri` | **Fixed** — parent endpoint EIP tracked; `uri:` lines show `[to]` etc. | | 7 | Index reads disk; dirty buffer line mapping wrong in edit mode | **Deferred** — same limitation as existing `g` goto; follow-up if needed | | 8 | `buildNodeLabel` walked into nested children | **Mitigated** — searches descendants but prefers descriptive props | | 9 | Tests didn't cover failure modes | **Fixed** — added tests for processor id, multi-route reset, beans exclusion, duplicate id filter | | 10 | Redundant structural filter logic | Accepted — kept for clarity | ### Nice-to-have (deferred) - Unify route parsing with `SourceTab.scanYamlRoutes` - Real nested tree for choice/when/doTry blocks - Remove FQCNs in `GotoSourceNodePopup` render (matches existing `GotoNodePopup` pattern) - Scan edit buffer instead of disk when in edit mode --- ## What Looks Good (Grok) - Ctrl+G vs `g` separation and footer/help text wiring - Popup UX mirrors `GotoRoutePopup` / `GotoNodePopup` (filter, paging, scrollbar) - `SourceViewer.goToLine()` edit-mode cursor positioning - Inline `to`/`toD`/`setHeader` scanning; multi-route files work with unique ids --- ## Test Coverage ```bash ./mvnw -pl dsl/camel-jbang/camel-jbang-plugin-tui test \ -Dtest=YamlRouteNodeScannerTest,GotoSourceNodePopupTest,SourceViewerGoToLineTest ``` | Test class | Cases | |------------|-------| | `YamlRouteNodeScannerTest` | Simple route+steps, inline from, multi-route, processor id scoping, route id reset, beans exclusion, structural key filtering | | `GotoSourceNodePopupTest` | Esc close, enter selection, arrow nav, fuzzy filter, duplicate route id filter independence | | `SourceViewerGoToLineTest` | View-mode and edit-mode cursor positioning | **16 tests, all passing.** --- ## Commits 1. `fb63168` — Initial implementation (scanner, popup, SourceTab/SourceViewer wiring, tests) 2. `d5170133` — Review fixes (id scoping, routeFromLine grouping, endpoint type on uri lines, dirty guard, expanded tests) -- 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]
