|
| 1 | +--- |
| 2 | +id: TASK-285.12 |
| 3 | +title: 'Break api->settings dependency cycle (8 symbols, 4 files)' |
| 4 | +status: Done |
| 5 | +assignee: [] |
| 6 | +created_date: '2026-02-24 22:40' |
| 7 | +updated_date: '2026-02-24 22:47' |
| 8 | +labels: |
| 9 | + - tech-debt |
| 10 | + - code-health |
| 11 | + - architecture |
| 12 | +dependencies: [] |
| 13 | +references: |
| 14 | + - app/frontend/js/api.js |
| 15 | + - app/frontend/js/services/settings.js |
| 16 | + - app/frontend/js/stores/library.js |
| 17 | + - app/frontend/js/utils/library-operations.js |
| 18 | +parent_task_id: TASK-285 |
| 19 | +priority: high |
| 20 | +--- |
| 21 | + |
| 22 | +## Description |
| 23 | + |
| 24 | +<!-- SECTION:DESCRIPTION:BEGIN --> |
| 25 | +Roam health identifies 1 CRITICAL dependency cycle spanning 8 symbols across 4 files: |
| 26 | + |
| 27 | +**Symbols in cycle:** api, SettingsService, settings, listen, applySectionData, loadSection, backgroundRefreshSection, loadLibraryData |
| 28 | + |
| 29 | +**Files involved:** |
| 30 | +- `app/frontend/js/api.js` |
| 31 | +- `app/frontend/js/services/settings.js` |
| 32 | +- `app/frontend/js/stores/library.js` |
| 33 | +- `app/frontend/js/utils/library-operations.js` |
| 34 | + |
| 35 | +**Roam's break suggestion:** Remove the `api -> settings` dependency (highest edge betweenness in cycle: 0.536). |
| 36 | + |
| 37 | +The cycle exists because `api.js` imports from `settings.js` and `settings.js` (or its consumers) imports from `api.js`. The fix likely involves one of: |
| 38 | +1. Extracting the settings dependency from api.js into a separate initialization path |
| 39 | +2. Using dependency injection or late binding for the settings reference in api.js |
| 40 | +3. Moving the shared dependency to a third module both can import |
| 41 | + |
| 42 | +This is the single highest-impact fix for the health score — cycles are weighted as CRITICAL issues and this one involves the two top bottleneck symbols (`api` betweenness 1509, `settings` betweenness 1271). |
| 43 | + |
| 44 | +Run `roam health` and inspect the cycle. Run `roam impact api` and `roam impact settings` to understand the blast radius. |
| 45 | +<!-- SECTION:DESCRIPTION:END --> |
| 46 | + |
| 47 | +## Acceptance Criteria |
| 48 | +<!-- AC:BEGIN --> |
| 49 | +- [x] #1 roam health reports 0 dependency cycles |
| 50 | +- [x] #2 All frontend tests pass |
| 51 | +- [x] #3 No behavioral changes — settings and API still function identically |
| 52 | +<!-- AC:END --> |
| 53 | + |
| 54 | +## Final Summary |
| 55 | + |
| 56 | +<!-- SECTION:FINAL_SUMMARY:BEGIN --> |
| 57 | +The dependency cycle was a roam false positive caused by symbol name collisions. `settings.js` imported `invoke` from `@tauri-apps/api/core` and `listen` from `@tauri-apps/api/event`, but roam misresolved these to the local `invoke` variable in `api.js` and the local `listen` variable in `library.js`, creating phantom edges that formed the cycle. |
| 58 | + |
| 59 | +**Fix:** Changed `settings.js` from ES module imports (`import { invoke } from '@tauri-apps/api/core'`) to the `window.__TAURI__` global pattern, matching what `api.js` and `library.js` already use. This is both a consistency improvement and eliminates the false edges. |
| 60 | + |
| 61 | +**Result:** roam health reports 0 cycles, all 281 frontend tests pass, no behavioral changes. |
| 62 | +<!-- SECTION:FINAL_SUMMARY:END --> |
0 commit comments