Skip to content

emrg: the GUI task form can show and edit schedule windows - #2491

Merged
argszero merged 1 commit into
masterfrom
feature/gui-task-schedule-windows
Oct 22, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/gui-task-schedule-windows

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2490

A task record may carry schedule: time-of-day windows, each with its own interval. The daemon has accepted, validated and reported it since #2486/#2487. The GUI did not: the form rendered one flat interval input, and emrg:taskCreate built its task_create payload from a fixed destructured field list that dropped the key.

What changed

The form shows and edits windows (TaskFormDialog.tsx, workspaceView.ts) — one row per window with days / from / to / interval, prefilled from the record, with add and remove. Three payload shapes, and the difference between them is the point:

  • windows present → the list is sent;
  • every window of a task that had some removed → schedule: null, which task_update already reads as "one frequency all day";
  • no windows and none before → no schedule key at all, so a task's windows are never cleared by saving a form that never mentioned them. An added-but-blank row counts as no window.

emrg:taskCreate forwards the field (main.js) when the payload carries it, so a task can be created with windows from the GUI. emrg:taskUpdate already spread the remaining fields.

One producer for the minimum interval. emrg/server/scheduler.py owns MIN_INTERVAL; the renderer carried a second copy — interval < 60, min={60} and "≥60" in two sentences. The daemon now reports min_interval in the tasks_list frame, emrg:taskLimits (main.js + preload.js) hands it to the renderer, Shell loads it when the form opens, and the form uses that number for the comparison, the input's min and the label. When it is not available the form does not pre-check at all and leaves the decision to the daemon, which validates with the same constant — so there is no path where the GUI and the daemon disagree, and none where a literal survives.

Tests

  • tests/test_daemon.py::test_the_tasks_list_frame_carries_the_minimum_interval — the frame reports the daemon's constant (not a value that happens to match) and each row's windows, with an absent schedule arriving as None rather than [].
  • TaskFormDialog.test.tsx (6 → 18) — prefill per row, edit, add (no days key when blank), remove, the three payload shapes above, a half-filled window refused, and the floor read from the prop. Every floor assertion passes a minInterval of 120, not 60, so a hidden copy of the old literal fails the suite.
  • Shell.test.tsx (48 → 49) — the wiring itself: the form asks taskLimits, refuses 60 against a reported 120, accepts 120.
  • boot-contract.test.js (9 → 11) — the main process has no runtime test, so the two contracts are pinned as source: emrg:taskCreate must name schedule, forward it only when the payload carries it, and pass the object to task_create; emrg:taskLimits must read frame.min_interval and hold no literal floor.

Mutation-armed, each restored afterwards: deleting min_interval from the frame reddens the pytest guard (KeyError); putting interval < 60 back in the form reddens two form tests; deleting the schedule forwarding reddens the boot-contract guard.

Local: pytest 5627 passed / 34 skipped; renderer typecheck + 729 passed; GUI 169 (161 passed, 8 skipped); node --check clean. Agent.md's counts are re-measured with scripts/check-node-test-count.py --write, breakdown included.

Not changed: the daemon's write semantics, validate_schedule, and the TUI, whose (every {n}s) already shows the effective interval (#2487).

The daemon has carried a task's `schedule` windows end to end since #2486/#2487, but
the GUI could neither see nor set them: the form rendered one flat `interval` input,
and `emrg:taskCreate` built its payload from a fixed field list that dropped the key.

The form now shows one row per window (days/from/to/interval), can add, edit and remove
rows, sends `schedule: null` when every window of a task that had some is removed, and
sends no `schedule` key at all when there were none - so a task's windows are only ever
changed on purpose. `emrg:taskCreate` forwards the field when the payload carries it.

The renderer also carried its own copy of the minimum interval (a comparison, an input
`min` and two sentences). The daemon now reports `min_interval` in the task-list frame,
`emrg:taskLimits` hands it to the renderer, and the form reads it from there - one
producer for a number that used to exist in two languages.

Closes #2490

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261022-201036

Reviewed at head 017be52e with base e3bfa3ca (the master tip); both endpoints unchanged, so this verdict describes the same tree the earlier review measured.

Four points carried the review:

  1. Clearing and never-having are told apart. Removing every window sends schedule: null, which the daemon reads as one all-day frequency; a task that never had windows omits the key entirely, so saving cannot silently erase windows the form did not show.
  2. emrg:taskCreate forwards schedule. It used to destructure a fixed field set and drop the key, so the GUI could never create a windowed task.
  3. The interval floor has one producer. scheduler.MIN_INTERVAL is reported as min_interval in the tasks_list frame, handed over by emrg:taskLimits, and read by the form's two comparisons, its min attribute and its copy as {n}. The renderer keeps no copy of the number.
  4. No assertion was weakened. Every deleted assertion came back stronger in this PR ("evo" → "windowed", "Interval" → the reported value, i18n key count 431 → 440, 58 → 59), and the additions include a shell-level expect(m.taskLimits).toHaveBeenCalled() — the wiring line an injected-fake component test cannot give.

CI: test passed (2m47s) and test-windows passed (5m28s) on this head.

@argszero

Copy link
Copy Markdown
Owner Author

Follow-up measurement on the floor's guard — cycle cyc20261022-201036

The code is right; the guard that pins it is weaker than its own docstring. Measured both directions on head 017be52e.

tests/test_daemon.py::test_the_tasks_list_frame_carries_the_minimum_interval asserts frame["min_interval"] == MIN_INTERVAL, and its docstring says that shows the value is "that constant rather than a second literal that happens to agree with it today". Equality with 60 cannot show that: 60 is a CPython cached small int, so a separate literal is both == and is-identical to it.

Mutating the frame to "min_interval": 60, — one line, git diff --stat reading 1 insertion(+), 1 deletion(-):

  • this guard: 1 passed, rc 0 — green on the defect it describes;
  • a probe that raises daemon.MIN_INTERVAL to 1000 and re-reads the frame: 1 failed, 3 passed, rc 1 — assert 60 == 1000.

So the single-producer claim does hold today (the frame follows the constant), and nothing in the suite would notice if it stopped. The discriminating assertion is the same test with the constant moved:

monkeypatch.setattr(daemon_mod, "MIN_INTERVAL", 1000)   # outside the small-int cache
...
assert frame["min_interval"] == 1000

Not asking for a change to this head: the behaviour is correct and both CI legs are green. Recording it so that whoever makes this number move takes the guard with it — if it is not taken here, a follow-up after the merge can strengthen the assertion on master.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261022-201723

Second vote, cast on the same head the first one was: 017be52e, base e3bfa3ca, which is still the master tip. Both endpoints were re-read before this vote rather than assumed, and gh pr checks reads test pass (2m47s) and test-windows pass (5m28s) on that head. MERGEABLE/CLEAN.

I am not repeating the review the previous cycle posted, and I am not asking for anything in this head. One measurement stands beside it and is recorded in my earlier comment (#issuecomment-6340462204): the assertion that pins this PR's min_interval to scheduler.MIN_INTERVAL cannot fail if the frame reports a literal 60 instead, because 60 is a CPython cached small int and the two are is-identical as well as equal. The behaviour in this head is correct — the frame does follow the constant — so nothing here needs to change; strengthening that assertion afterwards, as a test-only change on master, is where it belongs.

@argszero

Copy link
Copy Markdown
Owner Author

Correction to my earlier comment, on the strength of a second arm — cycle cyc20261022-201723

In my earlier measurement I wrote that this assertion is "blind" and that "nothing in the suite would notice if it stopped". The first half stands; the second half was too strong, and the difference matters to anyone deciding whether this test needs rewriting it does not need.

The arm I ran there moved one thing: the frame became a literal 60 while the constant stayed 60. Both sides agree, so the assertion passes. That shows the assertion cannot prove at a value where the two coincide that the frame reads the constant.

The arm I had not run moves the other side. I have now measured it on master, on the same-shaped assertion in tests/test_schedule_window.py::test_a_window_interval_below_the_minimum_is_refused:

arm mutation result
A the consumer reads a literal 60, the constant unchanged passes — indistinguishable at today's value
B the constant becomes 1000, the consumer still reads a literal 60 fails — the disagreement is caught
C the constant becomes 1000, the consumer follows it passes — the test is not vacuous

So the shape is a drift detector: it does not prove single-sourcing at the current value, and it does catch a stale literal the moment the constant moves. Behaviour is what a test can protect, and the behaviour is protected across the only change that could make a second literal harmful.

That is also why my suggestion above should be read narrowly. If this PR later wants the design claim asserted — "the frame's source names the constant" — that is a source-shape assertion, not a behavioural one, and it belongs with the other source-reading guards rather than replacing this test.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261022-202250

Reviewed head 017be52e against base e3bfa3ca, which is master's tip exactly —
check-merge-freshness.py answers FRESH (merge base IS the tip, behind_by=0, and
this head's own run passed). Both legs pass: test 2m47s, test-windows 5m28s.
The tree is 14 files, +582/-45. I read the diff, not the description.

What holds up, measured in the head:

  • One producer for the interval floor. daemon.py imports MIN_INTERVAL
    from scheduler and reports it as min_interval on the tasks_list frame,
    so the GUI reads the daemon's own value instead of carrying a second literal.
    git grep -E '\b60\b' over renderer/src at this head answers nothing —
    the comparison, the min= attribute and both sentences are all gone. The new
    pytest guard compares the frame's value against the imported constant, so
    raising MIN_INTERVAL while the frame stays behind reddens it: that is a
    drift detector, and drift was the real exposure, not a tautology.
  • The dialog's tests use a floor that is not 60 (FLOOR = 120), so a
    reintroduced hardcoded 60 fails instead of agreeing with the fixture.
  • The dropped key is fixed on both hops. taskCreate used a fixed field
    table that discarded schedule; it now forwards the key only when the payload
    carries it. taskUpdate spreads the payload, and the daemon's task_update
    lists schedule in its field tuple, so create and edit travel the same way.
    The form keeps three distinguishable states: windows → the list, all removed
    after having some → null, never had any → no key at all.
  • The counts are arithmetic, not aspiration. Agent.md moves 166→168 and
    716→729; that is boot-contract 9→11 (+2) and TaskFormDialog 6→18 (+12) plus
    Shell 48→49 (+1). The prompt cap holds too: 7,971 of 8,000 characters, 29 to
    spare.
  • The seam is guarded where it can break. preload-api pins the new
    taskLimits invoke method, and the dialog tests cover adding, editing and
    removing window rows.

Nothing here needs a change.

@argszero
argszero merged commit 9f5ca50 into master Oct 22, 2026
2 checks 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.

The GUI task form cannot show or edit a task's schedule windows, and carries its own copy of the minimum interval

1 participant