Fix manual reordering: dedupe context cache paths and stabilize order comparator - #555
Open
theslyprofessor wants to merge 1 commit into
Open
Fix manual reordering: dedupe context cache paths and stabilize order comparator#555theslyprofessor wants to merge 1 commit into
theslyprofessor wants to merge 1 commit into
Conversation
… comparator Manual reordering of space items breaks because the cached paths array that rank is derived from gets corrupted by two compounding bugs: 1. parseContextTableToCache appended paths twice: orderStringArrayByArray already returns every input path (unmatched ones included), and missingPaths were then appended again, so any child not matched in the db rows appeared twice in ContextState.paths. Raw row values were also compared without resolvePath, while mergeContextRows resolves them, so tables storing relative or stale paths never matched and every child was permanently duplicated. Now contextPaths are resolved with resolvePath and only paths present in contextPaths are ordered, with missing paths appended once. 2. orderStringArrayByArray used an inconsistent comparator: when both items were unranked it fell through to -1, reversing the unmatched run and producing different results on every sort. It now returns 0 for two unranked items, keeping their original relative order. Together these made rank (ranks.indexOf in getSpaceItems) live in a different index space than the actual db row array a drop inserts into (reorderRowsForPath), so drops landed in mirror-image positions and the order mutated again on the next reindex or on restart. Two smaller fixes on the same path: - mergeContextRows now dedupes rows by resolved path, so a table that already picked up duplicate rows heals on merge. - updateContextValue used `if (rank)`, silently ignoring rank 0, so a drop at the very top of a list was never persisted. Now `rank != null`. Fixes Make-md#443 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NPvX1eLWMjqdFwEYbvzjM
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect and unstable manual reordering in spaces by repairing how context-table paths are cached and how “unranked” items are compared during ordering, so drag/drop rank calculations align with the underlying .space/context.mdb row order.
Changes:
- Fixes
parseContextTableToCacheso cachedContextState.pathsis resolved consistently and no longer double-appends missing paths. - Stabilizes
orderStringArrayByArraysorting for unranked items to avoid reversing / non-deterministic ordering. - Heals existing corrupted context tables by deduping merged rows and ensures rank
0is persisted when updating context values.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/shared/utils/array.ts | Adjusts ordering comparator for unranked items to stabilize manual ordering. |
| src/core/utils/contexts/linkContextRow.ts | Dedupes merged context rows by resolved path to heal duplicated DB rows. |
| src/core/utils/contexts/context.ts | Persists rank 0 by treating rank as present when rank != null. |
| src/core/superstate/cacheParsers.ts | Resolves DB row paths consistently and prevents double-append duplication in cached path arrays. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
51
to
+55
| return array.sort( function (a, b) { | ||
| const A = order.indexOf(a), B = order.indexOf(b); | ||
|
|
||
| if (A == -1 && B == -1) { | ||
| // neither is ranked: keep original relative order (stable sort) | ||
| return 0 |
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.
Fixes #443
Full diagnosis with reproduction details is in #443 (comment). Summary:
Manual order is the row order of the
filestable in.space/context.mdb, and rank is derived at runtime asindexOfagainst a cached paths array. Two compounding bugs corrupt that array:Double-append in
parseContextTableToCache(src/core/superstate/cacheParsers.ts):orderStringArrayByArrayreturns the entirepathsarray (unmatched paths included), andmissingPathswas then appended again, so any child not matched in the db rows appeared twice inContextState.paths. The raw row values were also compared withoutresolvePath(whilemergeContextRowsdoes resolve them), so tables storing relative or stale paths never matched: every child was then permanently duplicated on every reindex. Fixed by resolvingcontextPathsviaresolvePathand ordering only the paths actually present incontextPaths, then appendingmissingPathsonce.Inconsistent comparator in
orderStringArrayByArray(src/shared/utils/array.ts): when both items were unranked (indexOf == -1for both) the comparator fell through to-1, which is not a valid consistent comparator, so the unmatched run came out reversed and could differ between sorts. Fixed to return0so unranked items keep their original relative order.Together these put the derived rank (
ranks.indexOfingetSpaceItems) in a different index space than the actual db row array a drop inserts into (reorderRowsForPath), which is why drops land in the wrong place and why the order mutates again after reopening Obsidian.Two smaller fixes on the same code path:
mergeContextRows(src/core/utils/contexts/linkContextRow.ts) now dedupes rows by resolved path, so a context table that already picked up duplicate rows heals on merge.updateContextValue(src/core/utils/contexts/context.ts) usedif (rank), silently ignoring rank 0, so a drop at the very top of a list was never persisted. Nowrank != null.The cache self-heals on the next reindex once the double-append is gone, and duplicate db rows heal via the merge dedupe.
Verification:
node esbuild.config.mjs productionbundles cleanly with no errors.tsc -noEmit -skipLibCheckoutput is byte-identical before and after this change; the single error it reports (TS1501 in src/adapters/text/textCacher.ts) is pre-existing on cleanmainand not touched by this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_015NPvX1eLWMjqdFwEYbvzjM