Skip to content

fix: honor documented repeater cookie semantics#12523

Merged
tiensonqin merged 5 commits into
logseq:masterfrom
crd:fix/recurring-task-advance-from-completion
May 14, 2026
Merged

fix: honor documented repeater cookie semantics#12523
tiensonqin merged 5 commits into
logseq:masterfrom
crd:fix/recurring-task-advance-from-completion

Conversation

@crd
Copy link
Copy Markdown
Contributor

@crd crd commented Apr 20, 2026

Summary

Logseq's documented repeater semantics (logseq/docs Tasks.md) define three org-mode-style cookies for recurring tasks:

  • .+ - repeats from the last completion date.
  • ++ - advances from the original scheduled date, skipping in whole intervals to the future.
  • + - advances from the original scheduled date once (can stack overdue).

The scheduler in worker/commands.cljs has been ignoring the cookie entirely and applying a single, ++-like semantic for every recurring task. This PR makes it honor the cookie.

What users see today vs. after this change

Scheduled 2026-04-01 weekly, DONE on 2026-04-05:

Cookie user wrote Docs say next occurrence should be Old behavior returned New behavior returns
.+1w 2026-04-12 (week from completion) 2026-04-08 2026-04-12
++1w 2026-04-08 (next future same-weekday) 2026-04-08 2026-04-08 (unchanged)
+1w 2026-04-08 (original + 1 week) 2026-04-08 2026-04-08 (unchanged)

No existing task's behavior changes on upgrade: tasks without an explicit cookie default to :double-plus (= ++), and the ++ implementation is mathematically equivalent to the previous scheduler. The win is that users can now select .+ (or +) and actually get that semantic.

What changed

  1. New closed-values property :logseq.property.repeat/repeat-type in deps/db/src/logseq/db/frontend/property.cljs, with values :dotted-plus / :plus / :double-plus. Default is :double-plus.

  2. Scheduler branches on repeat-type in src/main/frontend/worker/commands.cljs:

    • advance-from-completion (.+) - now + frequency * unit.
    • advance-from-scheduled (+) - original + frequency * unit, stacking allowed per org-mode.
    • advance-until-future (++) - advance from original in whole intervals until strictly after now.
  3. Non-positive frequency short-circuits to nil - the old code silently produced nonsense, and the new ++ loop would have spun forever. The guard is in get-next-time at the dispatch site.

  4. UI selector in the repeat-setting popover (src/main/frontend/components/property/value.cljs) - a dropdown for picking the cookie, shown when the repeat toggle is on. This restores the UI affordance that was removed in 0a5b88467 (Nov 2020, "Don't show repeat kinds selection"). For the past ~5.5 years the in-code support for all three cookies has been present but not user-pickable.

  5. New technical spec doc docs/recurring-tasks.md - restates and augments the upstream Tasks.md description, adds a decision table for picking the right cookie, and documents the implementation surface. The canonical user-facing description stays in logseq/docs; this doc serves contributors and power users.

Tests

src/test/frontend/worker/commands_test.cljs:

  • A new deftest per cookie (dotted-plus-advances-from-completion-test, plus-advances-from-scheduled-test, double-plus-advances-until-future-test) covering the distinctive behavior of each.
  • repeat-type-defaults-to-double-plus-test - unknown/missing repeat-type falls back to ++.
  • get-next-time-rejects-non-positive-frequency-test and get-next-time-rejects-unknown-unit-test - boundary safety.
  • dotted-plus-frequency-greater-than-one-test and double-plus-month-and-year-test - broader coverage.
  • The preexisting get-next-time-test passes unchanged under the :double-plus default - confirming ++ behavior is preserved byte-for-byte.
  • Time is pinned via (with-redefs [t/now (fn [] now)] ...) for deterministic assertions.

Full yarn test run: 502 tests, 2355 assertions, 0 failures, 0 errors (post-rebase onto current upstream master).

Notes for reviewers

  • Default choice: :double-plus preserves current behavior for every existing recurring task. If you'd prefer :dotted-plus as the default (matching the folk-intuition interpretation of "weekly"), happy to change - it's a one-line edit plus the expected test assertion flips.
  • with-redefs on t/now is new to this codebase; grep confirms no prior use. cljs-time.core/now is a plain defn var and the pattern is safe in CLJS.
  • Scope intentionally excludes the file-graph → DB-graph exporter's handling of repeater tags - that path currently dissocs :block/repeated? during migration (see deps/graph-parser/src/logseq/graph_parser/exporter.cljs:477), which is a separate preexisting issue. The parser-level cookie preservation that would pair with that fix is deferred to its own PR.

Refs #7731, #11260, #6715, #8531.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 20, 2026

CLA assistant check
All committers have signed the CLA.

@crd crd force-pushed the fix/recurring-task-advance-from-completion branch from 9717ef4 to 71eeb65 Compare April 22, 2026 19:22
crd added a commit to crd/logseq that referenced this pull request Apr 22, 2026
When importing a file-graph that contains recurring tasks (SCHEDULED or
DEADLINE timestamps with `.+1w` / `+1w` / `++1w` cookies), the parser
identifies the repeat but the exporter drops it — the migrated DB-graph
block is left without any of the `:logseq.property.repeat/*` properties,
so the task never reschedules in the new graph.

This change:

  * Extends `timestamps->scheduled-and-deadline` to emit
    `:repeat-type` / `:recur-unit` / `:recur-frequency` keywords on the
    block map, alongside the existing `:repeated? true`.
  * Extends `update-block-deadline-and-scheduled` in the exporter to
    translate those into `:logseq.property.repeat/repeated?`,
    `:logseq.property.repeat/repeat-type`, and
    `:logseq.property.repeat/recur-unit` on the DB-graph block.

Scope notes:

  * `:logseq.property.repeat/recur-frequency` is a `:number` property
    stored via a property-value block; wiring that through the exporter
    needs additional tx construction and is deferred. Migrated
    recurring tasks pick up the schema's default frequency of 1 via the
    scheduler's `resolve-recur-frequency` fallback, which is correct
    for every cookie that doesn't specify a multiplier.
  * Depends on the scheduler-property PR (logseq#12523) — the
    `:logseq.property.repeat/repeat-type` and `:recur-unit` closed-value
    schemas are declared there. This branch assumes logseq#12523 lands first.
    No schema-presence guard is kept in this patch because the
    dependency is a hard precondition, not a soft fallback.

Tests:

  * Parser-level `deftest timestamps-preserve-repeater-metadata` covers
    each cookie → `:repeat-type` keyword, each unit → `:recur-unit`
    keyword, the fallback for unknown kinds, frequency pass-through,
    and the non-recurring case.
  * Exporter-level assertions extend the existing "deadline and
    scheduled" fixture and add three new fixture lines (`+1h`, `++2d`,
    `.+1w`) verifying the full migration produces the right
    `:logseq.property.repeat/*` properties on the DB-graph block, and
    that `:recur-frequency` is intentionally absent so the scheduler's
    default-value path applies.

Refs logseq#7731, logseq#11260, logseq#12523.
@crd
Copy link
Copy Markdown
Contributor Author

crd commented May 4, 2026

Bump -- happy to answer questions, adjust scope, or split this into smaller PRs if that helps review.

crd added a commit to crd/logseq that referenced this pull request May 4, 2026
When importing a file-graph that contains recurring tasks (SCHEDULED or
DEADLINE timestamps with `.+1w` / `+1w` / `++1w` cookies), the parser
identifies the repeat but the exporter drops it — the migrated DB-graph
block is left without any of the `:logseq.property.repeat/*` properties,
so the task never reschedules in the new graph.

This change:

  * Extends `timestamps->scheduled-and-deadline` to emit
    `:repeat-type` / `:recur-unit` / `:recur-frequency` keywords on the
    block map, alongside the existing `:repeated? true`.
  * Extends `update-block-deadline-and-scheduled` in the exporter to
    translate those into `:logseq.property.repeat/repeated?`,
    `:logseq.property.repeat/repeat-type`, and
    `:logseq.property.repeat/recur-unit` on the DB-graph block.

Scope notes:

  * `:logseq.property.repeat/recur-frequency` is a `:number` property
    stored via a property-value block; wiring that through the exporter
    needs additional tx construction and is deferred. Migrated
    recurring tasks pick up the schema's default frequency of 1 via the
    scheduler's `resolve-recur-frequency` fallback, which is correct
    for every cookie that doesn't specify a multiplier.
  * Depends on the scheduler-property PR (logseq#12523) — the
    `:logseq.property.repeat/repeat-type` and `:recur-unit` closed-value
    schemas are declared there. This branch assumes logseq#12523 lands first.
    No schema-presence guard is kept in this patch because the
    dependency is a hard precondition, not a soft fallback.

Tests:

  * Parser-level `deftest timestamps-preserve-repeater-metadata` covers
    each cookie → `:repeat-type` keyword, each unit → `:recur-unit`
    keyword, the fallback for unknown kinds, frequency pass-through,
    and the non-recurring case.
  * Exporter-level assertions extend the existing "deadline and
    scheduled" fixture and add three new fixture lines (`+1h`, `++2d`,
    `.+1w`) verifying the full migration produces the right
    `:logseq.property.repeat/*` properties on the DB-graph block, and
    that `:recur-frequency` is intentionally absent so the scheduler's
    default-value path applies.

Refs logseq#7731, logseq#11260, logseq#12523.
@crd crd force-pushed the fix/recurring-task-advance-from-completion branch from 3a84439 to 60897a2 Compare May 4, 2026 16:06
@msiep
Copy link
Copy Markdown

msiep commented May 5, 2026

I would love to see this fixed.

@crd crd force-pushed the fix/recurring-task-advance-from-completion branch from 207eb8c to bf69171 Compare May 11, 2026 20:43
@crd
Copy link
Copy Markdown
Contributor Author

crd commented May 11, 2026

Rebased onto latest master, folded #12532's remaining delta (:repeat-type carry through) into this PR. @tiensonqin ready for your review!

Logseq's documented repeater semantics (per docs.logseq.com and
`logseq/docs` `Tasks.md`) define three org-mode-style cookies for
recurring tasks:

  `.+`: repeats from the last completion date
  `++`: advances from scheduled, skipping in whole intervals to future
  `+`:  advances from scheduled by the declared interval (can stack)

The scheduler in `worker/commands.cljs` has been ignoring the cookie
entirely and applying a single, `++`-like semantic for every
recurring task. A user who wrote `.+1w` in markdown — expecting "a
week from when I actually finished" — silently got `++1w` behavior
("a week from the original scheduled date, skipped to future"),
which for a weekly task scheduled 2026-04-01 and completed on
2026-04-05 returns the next occurrence on 2026-04-08 rather than
the documented 2026-04-12.

This change:

  * Adds a closed-values `:logseq.property.repeat/repeat-type` property
    with values `:dotted-plus` / `:plus` / `:double-plus`. Default is
    `:double-plus` so existing recurring tasks see no behavior change
    on upgrade.
  * Rewrites the scheduler to branch on repeat-type and implement each
    semantic: `.+` anchors on now; `+` advances from original once (can
    stack overdue, per org-mode); `++` iterates in whole intervals
    until strictly after now. The `++` path is mathematically
    equivalent to the previous scheduler, so default-path behavior is
    preserved.
  * Guards against frequency <= 0 — the old code would silently produce
    nonsense and, under the new `++` loop, would spin forever. The
    guard short-circuits to `nil`.
  * Extracts `resolve-recur-frequency` and fixes the previous
    `(or [A B] [C D])` pattern in `compute-reschedule-property-tx` —
    any 2-vector is truthy in Clojure, so the default-value branch
    was unreachable and entities without an explicit
    `:recur-frequency` silently fell through to `frequency = nil`.
    `if-let` makes both branches reachable so default-to-1 actually
    works at migration time.
  * Restores the cookie-type selector that was removed from the
    date-time popover in `0a5b88467` (Nov 2020) — in-code support for
    all three cookies has been present but not user-pickable for the
    last ~5.5 years.
  * Adds `docs/recurring-tasks.md` — a technical spec for contributors
    and users that restates and augments the upstream `Tasks.md` text,
    adds decision guidance, and documents the implementation surface.
  * Extends the file-graph → DB-graph migration (built on top of
    `44d6bd49c4` "fix: preserve repeated schedule import") to also
    carry the cookie kind via a new `repeat-types` map in
    `graph-parser/exporter.cljs`, so an imported `.+1w` task lands in
    the DB-graph with `:repeat-type.dotted-plus` rather than picking
    up the `:double-plus` default. Test updated to assert this.
  * Adds deftests covering each cookie's distinctive behavior plus
    boundary cases (non-positive frequency, unknown unit, frequency > 1
    variants, `++` at month/year units, and both branches of
    `resolve-recur-frequency`).

The preexisting `get-next-time-test` passes unchanged under the
`:double-plus` default, preserving the existing regression contract.
Tests pin `t/now` via `with-redefs` for determinism.

Refs logseq#7731, logseq#11260, logseq#6715, logseq#8531. Folds in the small remaining delta
from logseq#12532 (now closed as superseded by `44d6bd49c4`).
@tiensonqin tiensonqin force-pushed the fix/recurring-task-advance-from-completion branch from bed32c5 to 182aaa3 Compare May 14, 2026 08:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates recurring task scheduling so Logseq honors org-mode repeater cookie semantics for .+, +, and ++, while preserving ++ as the default behavior for existing tasks.

Changes:

  • Adds a built-in repeat-type property with closed values and migration support.
  • Updates scheduler logic, importer/exporter handling, UI selector, translations, and tests.
  • Adds contributor-facing documentation for recurring task semantics.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/main/frontend/worker/commands.cljs Implements repeat-type based scheduling and frequency resolution.
deps/db/src/logseq/db/frontend/property.cljs Defines the new repeat-type property and closed values.
deps/db/src/logseq/db/frontend/schema.cljs Bumps schema version to 65.26.
src/main/frontend/worker/db/migrate.cljs Adds migration for the repeat-type property.
src/main/frontend/components/property/value.cljs Adds repeat-type selector to the repeat settings popover.
src/main/frontend/components/property/value.css Styles the repeat-type selector.
src/resources/dicts/en.edn Adds English labels for repeat-type UI.
src/resources/dicts/zh-cn.edn Adds Simplified Chinese labels for repeat-type UI.
deps/graph-parser/src/logseq/graph_parser/exporter.cljs Preserves parsed repeater cookie kind during export.
deps/graph-parser/test/logseq/graph_parser/exporter_test.cljs Covers exporter preservation of repeat-type values.
src/test/frontend/worker/commands_test.cljs Adds scheduler tests for all repeat semantics and edge cases.
src/test/frontend/worker/migrate_test.cljs Tests migration creation of the repeat-type property.
docs/recurring-tasks.md Documents recurring task semantics and implementation surface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/frontend/worker/commands.cljs Outdated
Comment on lines +137 to +139
(if (t/after? result now)
result
(t/plus result (recur-unit frequency)))))
Comment thread docs/recurring-tasks.md Outdated
Copy link
Copy Markdown
Contributor

@tiensonqin tiensonqin left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, QAed and it works great! 🚢 👍

I also added migration, i18n for "Next date".

tiensonqin and others added 2 commits May 14, 2026 17:37
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tiensonqin tiensonqin merged commit 2169a86 into logseq:master May 14, 2026
1 check passed
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.

5 participants