chore(board): post-review cleanups - #274
Merged
Merged
Conversation
Follow-up to the code-review passes on the breadcrumb + model-routing work: - Drop the REST `/path` fetch in SheetPanel that existed only to count ancestors for the decorative ghost-card stack after the breadcrumb moved out. Depth now comes from the off-scene chain length; a synced sub-page not in the replica simply shows no stack (purely cosmetic) instead of a `/path` round-trip. - Extract the shared, cycle-safe parentId walk into `model/parent-chain.ts` and route both `buildNodePath` (breadcrumb) and `buildNotePath` (off-scene) through it, so the two crumb builders can't drift. - Fix the stale model-routing plan doc (it predated the nitro + 3s-timeout changes): gpt-oss-120b keeps `:nitro`, and the timeout is 2 → 3.
Follow-up to the review of this PR: - Delete get-note-path.ts (useGetNotePath): dropping the REST /path fetch removed its last caller, so the module is now unreachable. Refresh the stale reference in the off-scene note-path docstring. - Add e.stopPropagation() to the sheet panel's H1 title input keydown so editing the title no longer leaks Esc (closes the sheet) or letter keys (tool shortcuts) to the window listeners — matching the breadcrumb's title input.
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
1 participant
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.
Follow-up cleanups surfaced by the recent
/code-review highpasses (findings #3, #4, #5). No behavior change beyond dropping one redundant network call.Changes
/pathfetch inSheetPanel(ci(github): publish github releases from workflow #3). After the unified breadcrumb took over the ancestor chain, the panel keptuseGetNotePathpurely to count ancestors for the decorativeSheetStackBackgroundghost cards — a full/pathround-trip per synced sub-page open, for a single integer.stackDepthnow comes from the off-scene chain length; a synced sub-page not in the local replica shows no stack (purely cosmetic).buildNodePath(breadcrumb) andbuildNotePath(off-scene note) were two near-identical cycle-safe walks. Extracted the shared walk intomodel/parent-chain.ts(generic over item shape via id/parentId accessors); both now project the shared chain into their own crumb type, so the cycle-safety can't drift.model-routing-deepseek-base.mdpredated the later changes: it said gpt-oss-120b usesopenai/gpt-oss-120bwith no:nitroand a2 → 5timeout. Corrected to:nitro(open-weight, multi-provider) and2 → 3.Test plan
npm run check-all: cleannpm run test:run: 1354 passed (existingbuild-node-path+use-off-scene-notetests cover the refactor)