Repository navigation
refactor: drop three write-only maps - #10607
Merged
johannesjo merged 2 commits intoOct 9, 2026
Merged
johannesjo merged 2 commits into
johannesjo merged 2 commits into
Conversation
- op-log validation: projectTaskMap is set per project and never read - sync-md: mdById is filled and never read (mdByTitle and the id set are) - doc-mode: lastWrittenTitles is set/cleared but refreshTaskRef never consults it; the echo guard is pendingTitleWrites. Comments that described the map as the echo check are dropped with it. Claude-Session: https://claude.ai/code/session_01W9Mk18mBNfkPfgg3hYweKq
Contributor
|
Hello there irontaek! 👋 Thank you and congrats 🎉 for opening your first PR on this project! ✨ 💖 We will try to review it soon! |
Collaborator
|
🔍 Reviewed – no blocking issues found at Thanks for the PR! This removes three write-only maps ( Nits
Automated review pass (Claude Code). Anything unclear or wrong – say so and I'll take a look. |
Contributor
Author
|
Thanks for the review — reworded the comment to |
Contributor
Preview Deployment
Branch: Deployed with Cloudflare Pages |
Collaborator
|
Thank you very much! <3 |
johannesjo
added a commit
that referenced
this pull request
Oct 9, 2026
…nt-desktop-backup-fail-b9ebaa * origin/master: (23 commits) fix(tasks): save pending note edit before switching task (#10623) refactor(sync): split conflict-resolution service by responsibility (#10621) test(electron): cover local REST API guards and backup restore #8735 (#10604) fix(task-repeat-cfg): skip deleted instances in the next tooltip #10516 (#10603) refactor: drop three write-only maps (#10607) fix(notes): insert checklist items reliably on repeated toolbar clicks (#10566) feat(search): include projects and tags as search results (#10223) (#10606) Revert "ci: cancel superseded PR workflow runs" ci: cancel superseded PR workflow runs ci(e2e-sync): skip sync suites on draft PRs docs(sync-server): address review of env and sync-nav follow-ups docs(sync-server): align env.example, WebDAV caveat and sync nav docs(sync-server): fix SMTP/migration claims and review findings docs(sync-server): stop recommending master-sha pins in config comments docs(sync-server): clarify supported self-hosting path docs(agents): clarify E2E_WORKERS limits in e2e agent guide docs(agents): guide agent sessions on shared-machine e2e runs test(e2e): allow capping local workers via E2E_WORKERS fix(notes): keep hover controls clear of note text in the live editor (#10544) (#10605) feat(android): native deadline reminders up to 1 week before (#10619) ... # Conflicts: # electron/backup.test.cjs
johannesjo
added a commit
that referenced
this pull request
Oct 9, 2026
…ract-investigation-c4c2b9 * origin/master: (36 commits) fix(tasks): save pending note edit before switching task (#10623) refactor(sync): split conflict-resolution service by responsibility (#10621) test(electron): cover local REST API guards and backup restore #8735 (#10604) fix(task-repeat-cfg): skip deleted instances in the next tooltip #10516 (#10603) refactor: drop three write-only maps (#10607) fix(notes): insert checklist items reliably on repeated toolbar clicks (#10566) feat(search): include projects and tags as search results (#10223) (#10606) Revert "ci: cancel superseded PR workflow runs" ci: cancel superseded PR workflow runs ci(e2e-sync): skip sync suites on draft PRs docs(sync-server): address review of env and sync-nav follow-ups docs(sync-server): align env.example, WebDAV caveat and sync nav docs(sync-server): fix SMTP/migration claims and review findings docs(sync-server): stop recommending master-sha pins in config comments docs(sync-server): clarify supported self-hosting path docs(agents): clarify E2E_WORKERS limits in e2e agent guide docs(agents): guide agent sessions on shared-machine e2e runs test(e2e): allow capping local workers via E2E_WORKERS fix(notes): keep hover controls clear of note text in the live editor (#10544) (#10605) feat(android): native deadline reminders up to 1 week before (#10619) ... # Conflicts: # e2e/tests/task-dragdrop/task-multi-drag.spec.ts # packages/super-sync-server/README.md # packages/super-sync-server/docker-compose.yml # packages/super-sync-server/env.example
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
2 participants
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.
Problem
Three maps are populated but never read:
src/app/op-log/validation/is-related-model-data-valid.ts—validateTasksToProjectsAndTagsdoesprojectTaskMap.set(project.id, projectTaskSet)for every project; nothing readsprojectTaskMap(the validation usesprojectTaskSetdirectly). Never read since the file arrived with the pfapi → op-log move.packages/plugin-dev/sync-md/.../generate-task-operations.ts—mdByIdis filled next tomdByTitle, but onlymdByTitle,spById,spByTitleandduplicateIdsare used afterwards.packages/plugin-dev/doc-mode/src/ui/editor.ts—lastWrittenTitlesis set after a successful title write and cleared on context switch, butrefreshTaskRefnever consults it; the echo guard that actually runs ispendingTitleWrites. It has been this way since the plugin was added in 1c10ff6.Solution
Remove the three maps and the lines that fill them. For
lastWrittenTitlesthe doc comment and the.thencomment described it as the echo check inrefreshTaskRef, which it never was, so those sentences go with it. No behavior change.If you'd rather wire
lastWrittenTitlesintorefreshTaskRef(skip the refresh whentask.title === lastWrittenTitles.get(taskId)) instead of removing it, I'm happy to switch this PR to that.Type of Change
Checklist
npm run checkFileon changed.ts/.scssfiles — ran prettier 3.8--checkon the three files; full root install not donejest src/background/sync: 4 suites, 37 tests pass;tsc --noEmiterror counts in doc-mode/sync-md are unchanged from master (pre-existing, from missing built plugin-api types)type(scope): description)https://claude.ai/code/session_01W9Mk18mBNfkPfgg3hYweKq