chore: resolve issue #150 — resolve manager name when absent from loaded list - #153
Merged
Merged
Conversation
…om loaded list The employee list is loaded via listEmployeesWithRatings with the active filter/search and a 200-row cap, so a manager can be missing from it (e.g. filtered to active while the manager is on leave, or beyond the cap). The detail panel then fell back to rendering the raw manager ID. Add useResolvedManagerName: use the manager from the loaded list when present, otherwise fetch by ID via the existing get_employee command (no backend change). Returns undefined until resolved, so the raw ID shows only for a genuinely dangling reference. Hook is called before the early return to satisfy the Rules of Hooks. Verification: 72 frontend tests pass (4 new covering in-list, fetch-when-absent, no-manager, and fetch-failure fallback), tsc --noEmit clean. Resolves #150. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6 tasks
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #150 by ensuring the Employee detail panel can resolve a manager’s display name even when the manager isn’t present in the currently loaded (filtered + capped) employee list, by falling back to fetching the manager by ID via an existing Tauri command.
Changes:
- Adds a
useResolvedManagerNamehook that resolves from the loaded list when possible, otherwise fetches the manager by ID. - Updates
EmployeeDetailto use the new hook and pass the resolved manager name toDetailsSection. - Adds unit tests covering in-list resolution, fetch-when-absent, no-manager, and fetch-failure behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/components/employees/EmployeeDetail.tsx |
Uses the new hook to provide a resolved managerName to the details panel. |
src/components/employees/detail/useResolvedManagerName.ts |
Introduces the manager-name resolution hook with list-first + fetch fallback behavior. |
src/components/employees/detail/useResolvedManagerName.test.ts |
Adds tests for the hook’s resolution and fallback scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+40
| let cancelled = false; | ||
| getEmployee(managerId) |
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [managerId, managerInList]); |
Comment on lines
+54
to
+63
| mockCommands({ | ||
| get_employee: () => { | ||
| throw new Error('not found'); | ||
| }, | ||
| }); | ||
| const { result } = renderHook(() => | ||
| useResolvedManagerName('mgr-dangling', [emp({ id: 'emp-2' })]) | ||
| ); | ||
| await waitFor(() => expect(result.current).toBeUndefined()); | ||
| }); |
Switching between two employees whose managers both need fetch-resolution briefly showed the previous employee's manager name while the new request was in flight — the fetch branch never cleared the prior result. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
matthewod11-stack
added a commit
that referenced
this pull request
Jul 15, 2026
The backend rejects the streaming invoke with ChatError::Cancelled in addition to emitting chat-stream-cancelled. The catch block routed that rejection through categorizeError -> setMessageError, so hitting Stop decorated the partial message with a generic error + retry chip. Add isCancelledError (pure, tested) and a quiet-finalize branch in the catch, idempotent with the cancelled-event handler regardless of arrival order. Also merges current main (People Map T9 + #152 + #153) into the branch; integrated suite: 89 FE tests, tsc clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
matthewod11-stack
added a commit
that referenced
this pull request
Jul 15, 2026
…149) * chore: wire up frontend Stop button for streaming chat (#147) Issue #25 shipped the full backend cancel apparatus (StreamRegistry, cancel_stream command, chat-stream-cancelled event) but the frontend never wired it: no Stop button, and sendChatMessageStreaming never passed a stream_id, so the UI could never learn the id needed to cancel. Users could not stop a streaming response and abandoned streams billed to completion. - tauri-commands: sendChatMessageStreaming now forwards a client-generated streamId; add cancelStream() wrapping the existing cancel_stream command. - ConversationContext: generate a per-send stream id, listen for chat-stream-cancelled to reset streaming UI (backend emits this, not `done`, on cancel), expose stopStreaming(), and cancel on conversation switch/unmount so abandoned streams stop billing. - ChatInput: render a Stop button while streaming (optional isStreaming/onStop props — backward compatible; RecruitingView consumer unchanged). - App: wire stopStreaming + isStreaming into the chat ChatInput. - Extract the send/stop decision into a pure resolveChatInputMode() with unit tests (vitest 56 -> 63). Frontend-only; src-tauri/ untouched (honored do-not-touch). Verified: tsc --noEmit clean, vitest 63/63 green. Note: scope extended by one file beyond the issue's declared scope (src/App.tsx) — the sole consumer that wires the Stop gesture to the context; not in do-not-touch. Flagged in the PR for reviewer awareness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: don't render Stop as a failure (review finding) The backend rejects the streaming invoke with ChatError::Cancelled in addition to emitting chat-stream-cancelled. The catch block routed that rejection through categorizeError -> setMessageError, so hitting Stop decorated the partial message with a generic error + retry chip. Add isCancelledError (pure, tested) and a quiet-finalize branch in the catch, idempotent with the cancelled-event handler regardless of arrival order. Also merges current main (People Map T9 + #152 + #153) into the branch; integrated suite: 89 FE tests, tsc clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-generated by portfolio-orchestrator nightly run on 2026-07-15. Resolves #150.
What
The employee detail panel rendered the manager as a raw
ID: <manager_id>instead of the resolved name. Root cause:EmployeeContextloads employees vialistEmployeesWithRatings(filter, 200, 0)— filtered by the active search/filter and capped at 200 — soemployees.find(e => e.id === manager_id)legitimately misses when the manager is filtered out or beyond the cap.Fix
New
useResolvedManagerNamehook: use the manager from the loaded list when present; otherwise fetch by ID via the existingget_employeecommand (no backend change — honorsdo-not-touch: src-tauri/). Returnsundefineduntil resolved, so the raw ID only shows for a genuinely dangling reference (mirrors the backend's skip-not-fatal behavior). The hook is called before the component's early return per the Rules of Hooks.Verification
npm test— 72 passing / 0 failing (4 new: in-list resolution, fetch-when-absent (Employee detail rail shows manager as raw ID instead of resolved name #150's case), no-manager, fetch-failure fallback)npx tsc --noEmit— cleanrisk: low · approach: refactor-types · bail-if (backend signature change) not triggered — existing command reused.