fix: extend isMarkdownFilename across remaining session-start call sites#92
Merged
Conversation
PR #87 introduced the case-insensitive `isMarkdownFilename` helper for the new `listMarkdownSources` scanner but left three pre-existing call sites still using `endsWith(".md")` plus a case-sensitive `/\.md$/` strip: - `session-start.ts:208` — `brainIndex()` filter - `session-start.ts:212` — `brainIndex()` strip - `session-start.ts:258` — `listMd()` filter (vault file listing) An audit also caught two more in `formatActiveWork()` in the lib file (`lib/session-start.ts:34, 35`) — same module, same case- sensitive bug, same fix shape. Bundled here per PR #87's "two bugs, one PR" precedent. On NTFS/APFS case-insensitive filesystems a file saved as `Note.MD` or `note.Md` was silently dropped from the brain index, the active work section, and the vault file listing. After this fix all three sections route through the canonical helper and accept any case variant, matching how Obsidian itself surfaces the same file. New test in `formatActiveWork`'s suite locks `.MD`/`.Md`/`.md` acceptance and case-insensitive extension stripping. The integration test against the SessionStart hook continues to exercise the changed filters end-to-end. Closes #88 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the session-start markdown filename case-insensitivity fix by replacing remaining .md string checks with the canonical isMarkdownFilename() helper.
Changes:
- Updates
brainIndex()andlistMd()to include.MD/ mixed-case markdown files. - Makes extension stripping case-insensitive.
- Adds unit coverage for mixed-case active work filenames.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.claude/scripts/session-start.ts |
Applies case-insensitive markdown detection in brain index and vault listing. |
.claude/scripts/lib/session-start.ts |
Applies the helper and case-insensitive strip in active work formatting. |
.claude/scripts/tests/session-start.test.ts |
Adds mixed-case markdown extension coverage for active work formatting. |
Comments suppressed due to low confidence (1)
.claude/scripts/session-start.ts:258
- The case-insensitive vault listing path is still untested at the script level: the new test covers
formatActiveWork(), while the integration test's vault file listing only encounters lowercase markdown names. Add a mixed/uppercase root or nested.MDfixture and assert it appears in### Vault File Listingso this call site cannot regress independently of the helper tests.
else if (e.isFile() && isMarkdownFilename(e.name)) results.push(`./${full}`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…hook level The unit-level tests prove `isMarkdownFilename` accepts `.MD`/`.Md`, but they don't prove each call site actually delegates to that helper. Copilot's PR review and the /simplify quality agent both flagged this independently. Add an `Uppercase.MD` fixture to the integration test's synthetic vault `before()` and assert it surfaces in two places: - `### Brain Topics` section: bare `Uppercase` (case-insensitive strip via the new `/\.md$/i` regex). - `### Vault File Listing` section: `Uppercase.MD` (case-insensitive filter via `isMarkdownFilename`). A future refactor that bypasses the helper in either call site now fails this test, not just the unit tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
isMarkdownFilenamehelper for the newlistMarkdownSourcesscanner but three pre-existing call sites insession-start.tsstill usedendsWith(".md")and a case-sensitive/\.md$/strip. On NTFS/APFS (case-insensitive filesystems), a file saved asNote.MDornote.Mdwas silently dropped from the brain index, the active work section, and the vault file listing.lib/session-start.ts:34, 35insideformatActiveWork()— same module, same bug, same fix. Bundled per PR fix: stop SessionStart from flashing Obsidian app on macOS, close ExperimentalWarning stderr leak (#83) #87's "two bugs, one PR" precedent.Sites changed
session-start.tsbrainIndex()filtere.name.endsWith(".md")isMarkdownFilename(e.name)session-start.tsbrainIndex()stripf.replace(/\.md$/, "")f.replace(/\.md$/i, "")session-start.tslistMd()filtere.name.endsWith(".md")isMarkdownFilename(e.name)lib/session-start.tsformatActiveWork()filterf.endsWith(".md")isMarkdownFilename(f)lib/session-start.tsformatActiveWork()stripf.replace(/\.md$/, "")f.replace(/\.md$/i, "")isMarkdownFilenamewas already imported intosession-start.tsat line 36 — no import shuffle.Cross-platform behaviour
The change is pure-string filter and regex logic. No filesystem APIs touched.
Note.MDsaved by a different tool was silently dropped; now listed. No regression for lowercase files.Note.mdandNote.MDare distinct files; both are now listed instead of just lowercase. Cosmetic duplicate after extension strip; matches PR #87's precedent (isMarkdownFilenamealready accepts both).The new unit test passes synthetic filenames (no real FS) so behaviour is identical on every OS. Existing CI matrix exercises this on ubuntu / macos / windows × Node 22 / 24.
Test coverage
formatActiveWork's describe block (session-start.test.ts:73–80): locks.MD/.Md/.mdall pass the filter, extension stripped correctly, non-markdown file filtered out.isMarkdownFilenametests atsession-start.test.ts:306–331already cover uppercase / mixed-case / non-.md/ no-extension cases at the helper level. No change needed there.brainIndex()andlistMd()are filesystem-driven; their case-insensitivity is covered by the existing integration test plus the helper-level coverage.Out of scope (intentional)
lib/frontmatter.ts:34, 41— same kind ofendsWith(".md")in the validate-write hook. Different hook, different test surface. Worth a separate follow-up issue; not bundled here to keep the PR scoped to the session-start surface.Test plan
npm testin.claude/scripts/: 522/522 (was 521, +1)npm run typecheck: cleanbrain/Test.MDin a vault, run the hook, seeTestappear in the Brain Topics section🤖 Generated with Claude Code