Skip to content

fix(journal): correct ordinal suffix lookup and add regression tests#1

Merged
skridlevsky merged 1 commit into
skridlevsky:mainfrom
manfredlift:fix/journal-ordinal-suffix-rebased
Feb 15, 2026
Merged

fix(journal): correct ordinal suffix lookup and add regression tests#1
skridlevsky merged 1 commit into
skridlevsky:mainfrom
manfredlift:fix/journal-ordinal-suffix-rebased

Conversation

@manfredlift
Copy link
Copy Markdown
Contributor

Summary

Fixes journal_range page lookup for Logseq journals with ordinal date suffixes (for example Feb 15th, 2026) and adds regression tests.

Problem

JournalRange currently tries:

  • d.Format("Jan 2nd, 2006")

In Go time formatting, nd is treated as a literal string, so dates like the 15th are formatted as Feb 15nd, 2026 instead of Feb 15th, 2026.

As a result, valid Logseq journal pages are missed and entriesFound can incorrectly be 0.

Changes

  • Replaced inline date formatting in JournalRange with journalPageNames(d time.Time).
  • Added ordinalSuffix(day int) with correct suffix logic:
    • 11, 12, 13 -> th
    • otherwise: 1 -> st, 2 -> nd, 3 -> rd, default th
  • Preserved existing fallback formats:
    • 2006-01-02
    • January 2, 2006
  • Added tests in tools/journal_test.go:
    • TestOrdinalSuffix
    • TestJournalPageNames_UsesCorrectOrdinalSuffix

Result

journal_range now correctly matches standard Logseq journal names with ordinals, including pages like:

  • Feb 15th, 2026
  • February 15th, 2026

Notes

This PR is intentionally scoped to suffix generation and lookup behavior; it does not change query semantics beyond fixing date-name matching.

@manfredlift
Copy link
Copy Markdown
Contributor Author

@skridlevsky can you take a look when you get a chance?

@skridlevsky skridlevsky merged commit cddeffd into skridlevsky:main Feb 15, 2026
1 check passed
@skridlevsky
Copy link
Copy Markdown
Owner

@manfredlift Great catch - thanks for the fix! Merged.

donkeykong91 pushed a commit to donkeykong91/graphthulhu that referenced this pull request Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants