From bb879866f640af1b50eb979b5ac0570ce01e69a4 Mon Sep 17 00:00:00 2001 From: Aduneer <249940941+Aduneer@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:11:10 +0200 Subject: [PATCH 1/3] Build and test on macOS as well as Linux Everything outside the standard library is POSIX rather than Linux -- termios, ioctl, dirent, wcwidth -- and date.cpp and event.cpp have had their gmtime_r and localtime_r branches since the log landed. So this was expected to pass. Expected is not tested, and "runs on a Mac" is a claim the README should not make on the strength of reading the source. The runner is Apple Silicon, so this also builds for arm64, which is the half worth having beyond the platform itself: char is unsigned there and signed on x86. g++ on macOS is a shim for Apple clang rather than GCC, so that pair is excluded rather than run as a second clang under another name. Six jobs. --- .github/workflows/ci.yml | 20 ++++++++++++++++++-- CHANGELOG.md | 18 ++++++++++++++++++ README.md | 7 +++++-- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 214d319..57d84e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,8 @@ on: jobs: build-and-test: - # Two compilers times two build modes, so four jobs. + # Two platforms, two compilers and two build modes, minus a combination + # that would be a duplicate, so six jobs. # # Both compilers, so a warning or a standard-library difference that only # one of them notices still fails the build. @@ -18,11 +19,26 @@ jobs: # the optimised build is what ships, and the sanitised one is the only # thing that reports a read past the end of a buffer rather than returning # whatever was next to it. + # + # Both platforms, because everything the app does outside the standard + # library is POSIX rather than Linux -- termios, ioctl, dirent, wcwidth -- + # and "should be portable" is a claim nobody had checked. The macOS runner + # is Apple Silicon, so it also builds for arm64, where `char` is unsigned + # by default and x86 says it is signed. That is a real class of bug this + # cannot otherwise see. strategy: fail-fast: false matrix: + os: [ubuntu-latest, macos-latest] cxx: [g++, clang++] mode: [optimised, sanitised] + exclude: + # `g++` on a macOS runner is a shim for Apple clang rather than GCC, + # so this pair would run the clang entry again under another name. + # Real GCC is a `brew install` away; what the macOS runner is here to + # catch is the platform, not a third compiler. + - os: macos-latest + cxx: g++ include: # -Werror lives here rather than in the Makefile on purpose: a # warning should stop a change being merged, not stop a contributor @@ -40,7 +56,7 @@ jobs: -fsanitize=address,undefined -fno-sanitize-recover=undefined -fno-omit-frame-pointer - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} # Set for the job rather than passed to each make: the Makefile takes both # with ?=, and the test and golden targets have to be built with the same diff --git a/CHANGELOG.md b/CHANGELOG.md index 0de8943..08da212 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,24 @@ Notable changes per release. Dates are the release date; the PR numbers link the detail, which is where the reasoning lives. +## Unreleased + +### Changed + +- **CI builds and tests on macOS as well as Linux.** Everything FlashTerm does + outside the standard library is POSIX rather than Linux — `termios`, `ioctl`, + `dirent`, `wcwidth` — and the time functions have had their `gmtime_r` and + `localtime_r` branches since the log landed, so this was expected to pass. + Expected is not tested, and "runs on a Mac" is the kind of claim a README + should not make on the strength of reading the source. + + The macOS runner is Apple Silicon, so it also builds for arm64. That is the + half worth having beyond the platform itself: `char` is unsigned there and + signed on x86, which is a real difference that no amount of running on one + architecture can show. `g++` on macOS is a shim for Apple clang rather than + GCC, so that pair is excluded rather than run as a second clang under another + name — six jobs, not eight. + ## 0.3.0 — 2026-08-19 Pictures on cards, sync that puts two machines' reviews back together, and a CI diff --git a/README.md b/README.md index 974f10d..206ae55 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,10 @@ FlashTerm is a terminal flashcard app that makes you type the answer. No multipl ## Quick Start -Needs a C++17 compiler (`g++` or `clang++`) and `make`. +Needs a C++17 compiler (`g++` or `clang++`) and `make`. Linux and macOS are +both built and tested on every change; anything else POSIX will very likely +work, since nothing outside the standard library is used that POSIX does not +define. ```bash git clone https://github.com/Aduneer/FlashTerm @@ -633,7 +636,7 @@ cannot drive an app that insists on a tty. ## Contributing -Issues and pull requests are welcome. `make check` should pass before you open one; CI runs both suites on gcc and clang, in an optimised build and again under `-fsanitize=address,undefined`, all four with `-Werror`. Warnings are deliberately not errors in the Makefile itself, so a warning never stops you building — it stops the change being merged. +Issues and pull requests are welcome. `make check` should pass before you open one; CI runs both suites on Linux and macOS, on gcc and clang, in an optimised build and again under `-fsanitize=address,undefined`, all of them with `-Werror`. Warnings are deliberately not errors in the Makefile itself, so a warning never stops you building — it stops the change being merged. Released versions and what changed in them are in [CHANGELOG.md](CHANGELOG.md). From c90b2ab78e6e1441abc3a53b9b416d2ada5ba99d Mon Sep 17 00:00:00 2001 From: Aduneer <249940941+Aduneer@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:25:15 +0200 Subject: [PATCH 2/3] Make the seeded review order the same on every platform The macOS runner added in the previous commit failed on its first run: every multi-card golden case, and only those. The cause is std::shuffle, whose output the standard does not specify -- only that the permutation is uniformly random. libstdc++ and libc++ therefore deal the same seeded deck in different orders, and a scripted session answers cards in the order it expects to meet them, so every transcript recorded against libstdc++ was wrong on libc++. FLASHTERM_SEED is documented as fixing the review order. A promise that holds only on the standard library you happened to build against is not that promise, so the shuffle is now an explicit Fisher-Yates. mt19937 stays: the standard pins its algorithm exactly, down to a required value for the 10000th draw, so only the mapping from draws to indices needed taking back -- hence bounded(), by rejection sampling, since std::uniform_int_distribution is unspecified in the same way. Nothing is wrong with the randomness, and no real session changes meaningfully; the bug was in what the seed promised. One transcript changes on Linux, review-multi-card. Its input is remapped rather than simply re-recorded, so that the case still exercises what it was written for: the same correct, hint-then-partial, incorrect, correct, correct, landing on the cards the new order actually deals. --- CHANGELOG.md | 16 ++++++ src/review.cpp | 50 ++++++++++++++++++- tests/golden/cases/review-multi-card/expected | 30 +++++------ tests/golden/cases/review-multi-card/input | 4 +- 4 files changed, 81 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08da212..5be6ef6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ detail, which is where the reasoning lives. ## Unreleased +### Fixed + +- **`FLASHTERM_SEED` now fixes the review order on every platform, not just the + one you built on.** The shuffle went through `std::shuffle`, whose output the + standard does not specify — only that the permutation is uniformly random — + so libstdc++ and libc++ deal the same seeded deck in different orders. It is + now an explicit Fisher–Yates over `std::mt19937`, whose own output *is* + specified exactly, so the seed alone decides the order. + + Found by the macOS runner below, on its first run, which is the entire + argument for having added it: every multi-card golden case failed there, + because a scripted session answers cards in the order it expects to meet + them and the transcripts were recorded against libstdc++. Nothing was wrong + with the shuffle's randomness, and no real session is affected — the bug was + in what the seed promised. + ### Changed - **CI builds and tests on macOS as well as Linux.** Everything FlashTerm does diff --git a/src/review.cpp b/src/review.cpp index c83c0c7..c13dacb 100644 --- a/src/review.cpp +++ b/src/review.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -89,6 +90,51 @@ std::mt19937& rng() { return generator; } +// An index in [0, bound), drawn by hand rather than with +// std::uniform_int_distribution, whose mapping from generator output to result +// is not specified and does differ between standard libraries. See +// deterministic_shuffle. +// +// Rejection sampling: mt19937 covers the whole 32-bit range, so draws from the +// last, short block above the largest exact multiple of `bound` are discarded +// rather than folded in, which would make the low values fractionally likelier. +std::uint32_t bounded(std::uint32_t bound) { + const std::uint64_t range = std::uint64_t{1} << 32; + const std::uint64_t limit = range - (range % bound); + std::uint64_t draw; + do { + draw = rng()(); + } while (draw >= limit); + return static_cast(draw % bound); +} + +// Fisher-Yates, written out rather than calling std::shuffle. +// +// std::shuffle's output is not specified by the standard -- only that the +// result is a uniformly random permutation -- so libstdc++ and libc++ produce +// *different orders from the same seed*. FLASHTERM_SEED is documented as +// fixing the review order, and a promise that holds only on the standard +// library you happened to build against is not the promise it makes. The +// golden suite is what noticed: every multi-card case failed on macOS, because +// a scripted session answers cards in the order it expects to meet them, and +// the transcripts were recorded against libstdc++. +// +// mt19937 itself is safe to keep -- the standard specifies its algorithm +// exactly, down to a required value for the 10000th draw -- so pinning the +// permutation is only a matter of not letting the library choose how the +// numbers become indices. +template +void deterministic_shuffle(Iterator first, Iterator last) { + const std::size_t count = static_cast(last - first); + for (std::size_t i = count; i > 1; --i) { + const std::size_t pick = bounded(static_cast(i)); + if (pick != i - 1) { + std::iter_swap(first + static_cast(i - 1), + first + static_cast(pick)); + } + } +} + int clamp_box(int box) { return std::min(kMaxBox, std::max(1, box)); } bool card_has_any_tag(const Flashcard& card, @@ -273,7 +319,7 @@ void order_by_box(CardRefs* refs) { } refs->clear(); for (int box = 1; box <= kMaxBox; ++box) { - std::shuffle(boxes[box].begin(), boxes[box].end(), rng()); + deterministic_shuffle(boxes[box].begin(), boxes[box].end()); refs->insert(refs->end(), boxes[box].begin(), boxes[box].end()); } } @@ -651,7 +697,7 @@ void review_flashcards(Deck& deck) { return; } - std::shuffle(matches.begin(), matches.end(), rng()); + deterministic_shuffle(matches.begin(), matches.end()); if (session.mode == kModeBox && filters.leitner_box == 0) { order_by_box(&matches); } else if (filters.due_only) { diff --git a/tests/golden/cases/review-multi-card/expected b/tests/golden/cases/review-multi-card/expected index f8f1b67..7c3ab82 100644 --- a/tests/golden/cases/review-multi-card/expected +++ b/tests/golden/cases/review-multi-card/expected @@ -45,10 +45,10 @@ Next review in 3 days (). > Progress: [████████░░░░░░░░░░░░] 40% (2/5 cards) ┌──────────────────────────────────────────────────────────────┐ -│ Box 1 · new · french;greeting │ +│ Box 1 · new · french │ ├──────────────────────────────────────────────────────────────┤ │ │ -│ Bonjour │ +│ Au revoir │ │ │ └──────────────────────────────────────────────────────────────┘ @@ -56,14 +56,14 @@ Next review in 3 days (). Your answer: Progress: [████████░░░░░░░░░░░░] 40% (2/5 cards) ┌──────────────────────────────────────────────────────────────┐ -│ Box 1 · new · french;greeting │ +│ Box 1 · new · french │ ├──────────────────────────────────────────────────────────────┤ │ │ -│ Bonjour │ +│ Au revoir │ │ │ └──────────────────────────────────────────────────────────────┘ -Hint: H···· +Hint: G······ [Enter] submit [a] play audio [q] end session Your answer: ✅ Correct! @@ -75,16 +75,16 @@ Next review in 1 day (). > Progress: [████████████░░░░░░░░] 60% (3/5 cards) ┌──────────────────────────────────────────────────────────────┐ -│ Box 1 · new · french │ +│ Box 1 · new · french;animal │ ├──────────────────────────────────────────────────────────────┤ │ │ -│ Au revoir │ +│ Chien │ │ │ └──────────────────────────────────────────────────────────────┘ [Enter] submit [a] play audio [?] hint [q] end session Your answer: -❌ Incorrect! Correct answer: Goodbye +❌ Incorrect! Correct answer: Dog Next review in 1 day (). [Enter] next card [a] hear the question [e] edit this card @@ -110,10 +110,10 @@ Next review in 3 days (). > Progress: [████████████████████] 100% (5/5 cards) ┌──────────────────────────────────────────────────────────────┐ -│ Box 1 · new · french;animal │ +│ Box 1 · new · french;greeting │ ├──────────────────────────────────────────────────────────────┤ │ │ -│ Chien │ +│ Bonjour │ │ │ └──────────────────────────────────────────────────────────────┘ @@ -152,14 +152,14 @@ Next review in 3 days (). --- exit status --- 0 --- file deck.txt --- -Bonjour,Hello,french;greeting,0,1,1,,, +Bonjour,Hello,french;greeting,1,0,2,,, Merci,Thanks|Thank you,french,1,0,2,,, Au revoir,Goodbye,french,0,1,1,,, Chat,Cat,french;animal,1,0,2,,, -Chien,Dog,french;animal,1,0,2,,, +Chien,Dog,french;animal,0,1,1,,, --- file deck.txt.log --- ,,