refactor(storage): unify vault topic scanning into loadTopics boundary by Kuldeep2822k · Pull Request #98 · Kuldeep2822k/cli

Kuldeep2822k · 2026-08-17T08:27:51Z

Replaces repeated walkVault() + readFileSync() + parseFrontmatter() loops across CLI command handlers with a single, centralized loadTopics(vaultPath) boundary in src/storage/loader.ts.

Fixes #83

Changes

Added loadTopics(vaultPath) in src/storage/loader.ts returning normalized LoadedTopic[] (normalizing titles, aliases, dependencies, difficulties, scores, and SM-2 metadata).
Re-exported loadTopics and LoadedTopic in src/storage/index.ts.
Aligned TopicNode interface in src/types.ts with flat palee_schema: 1 properties.
Refactored dashboard.ts, migrate.ts, next.ts, plan.ts, progress.ts, review.ts, and validate.ts to consume loadTopics().
Added unit tests in test/storage-loader.test.ts.

Verification

npm run check: passing.
npm test: 170 passing tests.

…#83) - Implement loadTopics(vaultPath) in src/storage/loader.ts to centralize walkVault + frontmatter parsing + TopicNode normalization - Re-export loadTopics and LoadedTopic from src/storage/index.ts - Expand TopicNode and Topic data models in src/types.ts with typed scheduling/pillar fields - Refactor dashboard, migrate, next, plan, progress, review, and validate handlers to use loadTopics - Add unit tests in test/storage-loader.test.ts verifying normalization, aliases, and fallbacks

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

coderabbitai · 2026-08-17T08:28:06Z

Important

Review available on request

🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8361a876-ba71-4fa7-9e85-340ecbcc93fb

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

qodo-code-review · 2026-08-17T08:29:23Z

PR Summary by Qodo

refactor(storage): centralize vault topic parsing in loadTopics

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add a single loadTopics(vaultPath) boundary to scan/parse/normalize vault topics.
• Refactor multiple CLI commands to consume normalized LoadedTopic data.
• Expand TopicNode schema fields and add unit tests for loader normalization.

Diagram

graph TD
  A["CLI commands"] --> B["loadTopics()"] --> C["walkVault()"] --> D[("Vault markdown files")]
  B --> E["parseFrontmatter()"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Async + parallel file reads in loader

➕ Better performance on large vaults by overlapping IO
➕ Keeps CLI handlers simple while improving throughput
➖ More complexity (async plumbing across CLI)
➖ Potentially different error/ordering semantics than current synchronous reads

2. Schema-validated parsing (e.g., zod/io-ts) at the storage boundary

➕ Stronger guarantees for frontmatter types; clearer errors
➕ Central place to enforce palee_schema compatibility rules
➖ Adds dependency and validation overhead
➖ More boilerplate for a small CLI-focused codebase

3. Split loader into parse() + normalize() helpers

➕ Easier unit testing of normalization rules in isolation
➕ Clearer separation of concerns
➖ Slightly more surface area/files without major functional gain

Recommendation: The centralized loadTopics() boundary is the right default for consistency and reuse, especially since the CLI already used sync reads. Consider an async/parallel loader later only if vault size makes performance a user-visible issue; schema validation can be added incrementally if stricter migrations/compat checks become necessary.

Files changed (11) +305 / -179

Enhancement (2) +93 / -1

index.tsExpose loadTopics and LoadedTopic from the storage public API +4/-1

Expose loadTopics and LoadedTopic from the storage public API
• Re-exports the new loader boundary and its LoadedTopic type so CLI and other modules can consume a single entry point.
src/storage/index.ts

loader.tsAdd loadTopics() storage boundary with normalization and aliases +89/-0

Add loadTopics() storage boundary with normalization and aliases

• Introduces a centralized loader that walks the vault, reads file contents, parses frontmatter, and returns LoadedTopic objects for valid palee_id notes. Normalizes title fallback, dependency aliasing (dependencies -> depends_on), difficulty, mastery, status, and SM-2 metadata fields while preserving raw frontmatter/content for downstream use.

src/storage/loader.ts

Refactor (8) +123 / -178

dashboard.tsReplace per-file vault parsing with loadTopics mapping +20/-31

Replace per-file vault parsing with loadTopics mapping

• Removes direct walkVault/fs/parseFrontmatter usage and builds DashboardTopic from LoadedTopic. Keeps date parsing/invalid-date handling local while relying on loader normalization for defaults.

src/cli/dashboard.ts

migrate.tsScan schema versions via loaded topics +7/-18

Scan schema versions via loaded topics

• Switches migration scan from reading files directly to iterating loadTopics output. Uses the preserved raw frontmatter to read palee_schema and reports file paths from loader metadata.

src/cli/migrate.ts

next.tsCompute due topics from loadTopics output +15/-20

Compute due topics from loadTopics output

• Replaces manual vault walk + frontmatter parsing with loadTopics iteration. Adds difficulty to DueTopic and uses loader-provided path/title/mastery defaults.

src/cli/next.ts

plan.tsBuild planning TopicNode map from LoadedTopic +15/-22

Build planning TopicNode map from LoadedTopic

• Replaces manual frontmatter parsing with loadTopics and populates PlanTopic from normalized fields. Keeps due date parsing logic but relies on loader normalization for dependencies, mastery, and difficulty defaults.

src/cli/plan.ts

progress.tsGenerate progress summary from loaded topics +16/-26

Generate progress summary from loaded topics

• Converts progress topic collection into a simple map() over loadTopics output. Uses normalized SM-2 and assessment metadata with sensible null/default fallbacks.

src/cli/progress.ts

review.tsFind review candidates via loadTopics; keep write path correct +15/-37

Find review candidates via loadTopics; keep write path correct

• Replaces candidate discovery (walkVault + parseFrontmatter) with filtering LoadedTopic objects. Uses loader-provided content/frontmatter/filePath when updating and atomically writing the reviewed note.

src/cli/review.ts

validate.tsValidate dependency graph using loaded topic metadata +15/-24

Validate dependency graph using loaded topic metadata

• Uses loadTopics to populate the topic map and detect duplicate IDs, avoiding repeated filesystem reads/parsing. Continues to run validateDependencyGraph over the resulting TopicNode map.

src/cli/validate.ts

types.tsExpand TopicNode to include canonical schema fields and metadata +20/-0

Expand TopicNode to include canonical schema fields and metadata

• Extends TopicNode with optional fields used across the schema (title/path/difficulty, pillars, SM-2 scheduling, dates). Enables typed access to loader-populated properties without ad-hoc casting in CLI handlers.

src/types.ts

Tests (1) +89 / -0

storage-loader.test.tsAdd unit tests for loadTopics normalization and fallbacks +89/-0

Add unit tests for loadTopics normalization and fallbacks

• Creates a temp vault and asserts loader behavior for empty vaults, schema field extraction, filename title fallback, and dependency alias normalization. Verifies SM-2 fields and due date string extraction are surfaced consistently.

test/storage-loader.test.ts

…ort to loadTopics (#83) - Parse and clamp topic_mastery and assessment pillars [0.0, 1.0] with 4-decimal rounding - Support string numeric scores in frontmatter

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

qodo-code-review · 2026-08-17T08:35:54Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~validate.ts scans vault filesystem twice~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

validate.ts calls walkVault(vaultPath) directly to compute file_count and then calls
loadTopics(vaultPath), which internally calls walkVault(vaultPath) again, causing a redundant full
directory tree traversal (and duplicate file reads/parses) on every validate run.

Code

src/cli/validate.ts[R26-27]

    const files = walkVault(vaultPath);
+    const loaded = loadTopics(vaultPath);

Relevance

●●● Strong
Redundant vault traversal and rereading is a concrete inefficiency; reviewers accepted related
vault-scan correctness concerns.
PR-#54

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

validate.ts keeps the pre-existing const files = walkVault(vaultPath); call and adds `const loaded
= loadTopics(vaultPath);` right after it; loadTopics itself calls walkVault(vaultPath) internally
(src/storage/loader.ts line 27), so the vault directory tree is walked twice and every file is
read/parsed an extra time via loadTopics, even though files.length is only used for the
file_count field.

src/storage/loader.ts[26-28]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
validate.ts performs two full vault traversals per run: one explicit `walkVault(vaultPath)` call to get `files` (used only for `file_count`), and a second implicit traversal inside `loadTopics(vaultPath)`.

## Issue Context
`loadTopics` (src/storage/loader.ts) already calls `walkVault` and reads/parses every markdown file. validate.ts only needs `files.length` for reporting `file_count`, which does not require a second separate walk.

## Fix Focus Areas
- src/cli/validate.ts[26-27]
- src/storage/loader.ts[26-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~NaN review counters leak~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The loader accepts NaN as repetition and lapse counts because typeof NaN is number, and
refactored consumers retain it with ?? 0. A YAML .nan value consequently makes progress
aggregates NaN, which JSON serializes as null, whereas the previous value || 0 paths defaulted
it to zero.

Code

src/storage/loader.ts[R77-78]

+      repetition: typeof frontmatter.repetition === 'number' ? frontmatter.repetition : 0,
+      lapses: typeof frontmatter.lapses === 'number' ? frontmatter.lapses : 0,

Relevance

●●● Strong
NaN passes type-only numeric validation and contaminates aggregates; this is a deterministic
correctness bug with an obvious finite-number fix.
PR-#7

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The YAML library resolves .NaN under its default schema to JavaScript NaN. The loader's
type-only checks preserve that value; progress then retains it with nullish coalescing and adds it
into total_reviews and total_lapses, contaminating the aggregate.
src/storage/loader.ts[75-79]
src/cli/progress.ts[31-42]
src/cli/progress.ts[114-123]
🌐 The YAML scalar type documentation lists .nan as a float NaN value resolved by the default schema.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Numeric normalization accepts non-finite values such as YAML `.nan`, allowing `NaN` to contaminate command calculations and JSON output.

## Issue Context
Use `Number.isFinite` for repetition, lapses, and the other normalized numeric fields, falling back to their canonical defaults. Add a loader or CLI regression test using YAML `.nan`.

## Fix Focus Areas
- src/storage/loader.ts[55-79]
- src/cli/progress.ts[31-42]
- test/storage-loader.test.ts[19-88]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~loadTopics silently swallows unreadable files~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

loadTopics swallows every per-file fs.readFileSync failure with an empty catch { continue; },
silently skipping unreadable Markdown files (e.g., permission denied or a race with deletion) and
causing validation and migration to proceed over an incomplete vault with misleading “success”
outcomes. Because this loader is now centralized, all seven refactored CLI handlers that depend on
it lose visibility into partial scan failures and may omit inaccessible topics without any
diagnostic trace.

Code

src/storage/loader.ts[R32-36]

+    try {
+      content = fs.readFileSync(filePath, 'utf8');
+    } catch {
+      continue;
+    }

Relevance

●●● Strong
Accepted precedent flags unreadable vault data being treated as successful empty results;
centralized swallowing similarly hides incomplete scans.
PR-#54
PR-#7

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited behavior is centralized in src/storage/loader.ts, where per-file read failures are
caught and ignored, so any unreadable Markdown file is simply skipped. Since the seven refactored
commands (dashboard, migrate, next, plan, progress, review, validate) now consume loadTopics, a
single permission or transient I/O failure reduces the loaded topic set vault-wide without logging.
Downstream, validate computes valid only from errors found among the topics it successfully
loaded, and migrate derives its schema reporting solely from the loaded array, so a Markdown file
can still be discovered by a separate walk yet never parsed/validated, and migration has no signal
that its scan was incomplete.

src/storage/loader.ts[30-36]
src/storage/loader.ts[30-40]
src/cli/validate.ts[26-65]
src/cli/migrate.ts[15-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`loadTopics` currently hides `readFileSync` failures by catching and continuing, which silently skips unreadable Markdown files and makes callers treat an incomplete topic scan as authoritative; this can lead to misleading “successful” validate/migrate results and missing topics across all refactored CLI commands.

## Issue Context
This silent behavior is centralized in `src/storage/loader.ts` and affects every command that now uses the shared loader (dashboard, migrate, next, plan, progress, review, validate). Prior to the refactor, many of these flows would abort on file-read failures; now errors can be suppressed with no logging/diagnostics, so validate may report `valid` based only on the subset it loaded and migrate may produce schema reporting without realizing some files were never scanned. Prefer either (a) throwing an error that includes the affected path so commands fail loudly, or (b) returning explicit loader diagnostics (e.g., list of failed paths/errors) that every caller must report and treat as an incomplete scan.

## Fix Focus Areas
- src/storage/loader.ts[30-36]
- src/cli/validate.ts[26-65]
- src/cli/migrate.ts[15-52]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. review.ts loads and normalizes entire vault per query 🐞 Bug ➹ Performance

Description

reviewCommand now routes topic lookup through loadTopics(vaultPath), which reads, parses, and fully
normalizes every markdown topic in the vault (SM-2 fields, difficulty, dependencies, etc.) just to
filter down to the single topic matching topicQuery, adding avoidable per-file normalization
overhead on a command typically invoked once per review session.

Code

src/cli/review.ts[R23-29]

+    const loaded = loadTopics(vaultPath);
+    const candidates = loaded.filter(
+      (t) =>
+        t.palee_id === topicQuery ||
+        t.palee_id.includes(topicQuery) ||
+        t.title.toLowerCase().includes(topicQuery.toLowerCase())
+    );

Relevance

●● Moderate
Performance concern is plausible, but historical evidence favors centralized full-vault loaders;
query-specific optimization is subjective.
PR-#54

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

src/storage/loader.ts builds a fully-populated LoadedTopic (14+ normalized fields) for every file in
the vault; review.ts only needs id/title/content/frontmatter/filePath to find and update one topic,
so the extra normalization work (difficulty coercion, dependency array building, numeric coercions
for SM-2 fields) is wasted for all but one topic on every invocation.

src/storage/loader.ts[54-83]

Context

✅ Web pages:

🌐 Parse & Stringify

🌐 tests/doc/types.ts at main · eemeli/yaml

🌐 docs/04_documents.md

+2 more

Review mode: 🧠 Deep: This is a substantial behavioral refactor centralizing parsing across seven CLI paths, with normalization and data-model changes that create many independent, easy-to-miss compatibility risks.

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

qodo-code-review · 2026-08-17T08:35:56Z

+    const loaded = loadTopics(vaultPath);
+    const candidates = loaded.filter(
+      (t) =>
+        t.palee_id === topicQuery ||
+        t.palee_id.includes(topicQuery) ||
+        t.title.toLowerCase().includes(topicQuery.toLowerCase())
+    );


4. Review.ts loads and normalizes entire vault per query 🐞 Bug ➹ Performance

reviewCommand now routes topic lookup through loadTopics(vaultPath), which reads, parses, and fully normalizes every markdown topic in the vault (SM-2 fields, difficulty, dependencies, etc.) just to filter down to the single topic matching topicQuery, adding avoidable per-file normalization overhead on a command typically invoked once per review session.

… in validate (#83) - Use parseNumber and parseInteger to guard against YAML .nan / non-finite review counters - Pass scanned files array to loadTopics in validate.ts to prevent redundant second walk - Normalize vault walker to always start from resolved absolute path and exclude dot-files

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

Kuldeep2822k · 2026-08-17T09:27:56Z

@coderabbitai full review

coderabbitai · 2026-08-17T09:28:14Z

⚠️ Action not completed

Review rate limited.

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 8 minutes.

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

…tCode in review (#83) - Do not silently swallow readFileSync errors in loadTopics so callers fail loudly on unreadable files - Standardize reviewCommand to use process.exitCode instead of hard process.exit

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

…ing (#83) - Default status to 'not_started' when mapping LoadedTopic to ProgressTopic

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

greptile-apps

Kuldeep2822k has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

github-actions Bot added core Core source code changes tests Test suite changes labels Aug 17, 2026

qodo-code-review Bot added the 🕐 40+ Minutes label Aug 17, 2026

refactor(storage): add parseScore normalization and string score supp…

606019c

…ort to loadTopics (#83) - Parse and clamp topic_mastery and assessment pillars [0.0, 1.0] with 4-decimal rounding - Support string numeric scores in frontmatter

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

qodo-code-review Bot reviewed Aug 17, 2026

View reviewed changes

Kuldeep2822k changed the title ~~refactor(storage): unify vault topic parsing into loadTopics boundary~~ refactor(storage): unify vault topic scanning into loadTopics boundary Aug 17, 2026

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

Merge branch 'main' into refactor/issue-83-unified-load-topics

b1ec289

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

fix(storage): propagate read errors in loadTopics and use process.exi…

4b18271

…tCode in review (#83) - Do not silently swallow readFileSync errors in loadTopics so callers fail loudly on unreadable files - Standardize reviewCommand to use process.exitCode instead of hard process.exit

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

fix(cli): provide fallback for optional topic status in progress mapp…

f7acb34

…ing (#83) - Default status to 'not_started' when mapping LoadedTopic to ProgressTopic

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

Merge branch 'main' into refactor/issue-83-unified-load-topics

6573257

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

Kuldeep2822k merged commit b0167c8 into main Aug 17, 2026
11 checks passed

Kuldeep2822k deleted the refactor/issue-83-unified-load-topics branch August 29, 2026 17:33

This was referenced Aug 30, 2026

docs: bring CHANGELOG.md up to date with 0.3.0, 0.3.1, and Unreleased features #118

Merged

refactor(types): merge duplicate WalkOptions declaration; mark Topic/Assessment/Review as Phase-2 reserved #125

Closed

Conversation

Kuldeep2822k commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Verification

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review available on request

Uh oh!

qodo-code-review Bot commented Aug 17, 2026

PR Summary by Qodo

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Kuldeep2822k commented Aug 17, 2026

Uh oh!

coderabbitai Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Kuldeep2822k commented Aug 17, 2026 •

edited

Loading

coderabbitai Bot commented Aug 17, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 17, 2026 •

edited

Loading

coderabbitai Bot commented Aug 17, 2026 •

edited

Loading