Skip to content

feat(scheduling): add ReviewHistoryToEntries and simplify Reschedule for non-advanced users#63

Draft
cantalupo555 wants to merge 3 commits into
mainfrom
feat/simplify-reschedule
Draft

feat(scheduling): add ReviewHistoryToEntries and simplify Reschedule for non-advanced users#63
cantalupo555 wants to merge 3 commits into
mainfrom
feat/simplify-reschedule

Conversation

@cantalupo555
Copy link
Copy Markdown
Member

Summary

Adds ReviewHistoryToEntries() converter to simplify scheduling for non-advanced users, and improves Reschedule() with better validation, error handling, and documentation.

Motivation

Reschedule() was identified as overly complex for typical use cases. This PR provides a simpler alternative path using MemoryState()/HistoricalMemoryStates() while maintaining full backward compatibility. It also fixes a pre-existing bug where calculateManualRecord silently discarded errors.

Changes Made

  • memory.go: Added ReviewHistoryToEntries() to convert timestamp-based review history to deltaT-based entries
  • reschedule.go: Improved input validation, fixed error propagation in calculateManualRecord, and added documentation pointing to the simpler alternative
  • fsrs_test.go: Expanded test coverage for Reschedule edge cases
  • memory_test.go: Added TestReviewHistoryToEntries and cross-validation test confirming identical stability/difficulty results between both scheduling paths within 1e-6 tolerance

Testing

  • go test: all tests pass
  • go vet: no warnings
  • 13 new subtests covering edge cases, error paths, and cross-path consistency

Breaking Changes

None — this is purely additive. Existing Reschedule() behavior is unchanged; new ReviewHistoryToEntries() provides a simpler scheduling path.

Closes #53

@cantalupo555
Copy link
Copy Markdown
Member Author

@ishiko732 This was based on your suggestion.
Would you like any adjustments?

Comment thread fsrs_test.go
Comment on lines +3399 to +3404
r1 := time.Date(2024, 9, 13, 0, 0, 0, 0, time.UTC)
manualDue := time.Date(2024, 9, 28, 0, 0, 0, 0, time.UTC)
reviews := []ReviewHistory{
{Rating: Manual, Review: r1, State: StatePtr(Review), Stability: 10.0, Difficulty: 5.0, Due: manualDue},
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t want to make it that complicated. In fact, I haven’t seen anyone manually adjust DS.
The intended use of reschedule is to refresh a card’s next due time after the parameters have been updated.

Open-source projects with over 50 stars that depend on ts-fsrs don’t seem to use the reschedule method.

Comment thread memory.go
Comment on lines +78 to +81
//
// Returns an error if the list is empty, any review has a zero timestamp,
// any rating is [Manual] or outside the range [Again–Easy], or any computed
// DeltaT is negative (indicating out-of-order timestamps).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s correct.

Base automatically changed from feat/input-validation to main May 8, 2026 14:38
@cantalupo555 cantalupo555 marked this pull request as draft May 8, 2026 15:37
…eduling

- Introduces ReviewHistoryToEntries to convert timestamp-based []ReviewHistory into DeltaT-based []ReviewEntries
- Validates input: empty slices, zero timestamps, Manual ratings, invalid ratings, negative DeltaT
- Cross-validates against Reschedule to ensure identical stability/difficulty
…document simpler alternative

- Added input validation loop for rating range and zero review times
- Fixed calculateManualRecord to propagate errors instead of silently discarding them
- Updated godoc to recommend MemoryState + ReviewHistoryToEntries as simpler alternative
- Added 5 validation test cases covering invalid rating, zero review time, and manual entries
…tries edge cases

- Add single-entry and boundary rating tests for ReviewHistoryToEntries
- Add manual non-New as first review test verifying zero elapsed days
- Add all-manual RescheduleItem field assertions
- Add manual Learning/Relearning zero-stability fallback tests
- Add handleManualRating logDue fallback test for zero LastReview
- Add error message content verification test
@cantalupo555 cantalupo555 force-pushed the feat/simplify-reschedule branch from 7e4eb00 to ef46bb8 Compare May 9, 2026 11:40
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.

Simplify Reschedule by leveraging HistoricalMemoryStates for graded replay

2 participants