From 14c8938be8879bab8ff610abdcb12ceafbc6a821 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Thu, 17 Sep 2026 19:19:34 +0100 Subject: [PATCH 01/23] =?UTF-8?q?test(plan):=20RED=20for=20item=204=20?= =?UTF-8?q?=E2=80=94=20evidence-based,=20consensual=20completion=20(D-G)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven tests pin scenario 4's redesign before any production code changes: the completion action rule 9 emits for a fully-checked active plan must carry the end assessment (due reviews, struggles and unverified milestones counted on the plan's own concepts), propose `extend` while any count is above zero and `close` when all are zero, compose its sentence from that proposal, and never change a status — the read is `assess(AssessPlan(phase="end", record=False))`, the preview path, so the document, its status and the checkpoint log are untouched. A failed assessment keeps the pre-change sentence and adds a warning; `now` never fails on it. `plan close ` is the launch sibling of `plan repair`: the same architect chain with a `### Closing review` first section (three counts, proposal, evidence lines), refusing a plan with open milestones with their count. Owner decision 2026-09-17: the completion review counts only due rows that name a concept. The scheduler's "New topic -- start fresh" row (`concept: None`, `evidence: configured_topic`) is a cold-start hint for "what should I review now", not a lapsed review; the evaluator already ignores it at concept level (it never matches a milestone concept, so it contributes nothing to `unverified_milestones`), and counting it would tell a learner who has just ticked every milestone to "start fresh". `plan evaluate` keeps the row — the exclusion is the completion review's, applied by one definition in both the engine and the brief. Evidence is planted on the `studyloop.history` package attributes the evaluation resolves at call time, so the relevance filter and `has_evidence` logic stay live and no sessions database is involved. All seven fail for the intended reason (missing attributes, `assess` never called, no warning, no `close` command); the 57 pre-existing tests in both files pass and the no-plan golden sha is unchanged. The first test's T4.1 name lost one redundant word (`plan_concepts` → `concepts`): no def line in the repo exceeds 100 chars and none carries a `noqa`. The `pyright: ignore[reportAttributeAccessIssue]` tags on the not-yet-existing attributes are the 3b RED's pattern; GREEN strips them. --- .../studyloop/tests/test_cli_plan_seam.py | 132 +++++++++++ .../studyloop/tests/test_now_plan_guidance.py | 212 ++++++++++++++++++ 2 files changed, 344 insertions(+) diff --git a/packages/studyloop/tests/test_cli_plan_seam.py b/packages/studyloop/tests/test_cli_plan_seam.py index 598d8d94b..ecf57d0cc 100644 --- a/packages/studyloop/tests/test_cli_plan_seam.py +++ b/packages/studyloop/tests/test_cli_plan_seam.py @@ -700,3 +700,135 @@ def test_husk_refusal_names_both_pause_and_repair(runner, isolated_plans_dir) -> clean = _ANSI.sub("", result.output) assert "studyloop plan status husk paused" in clean assert "studyloop plan repair husk" in clean + + +# --------------------------------------------------------------------------- +# Item 4 (D-G) — `plan close `: the closing review is a launch, not a write +# --------------------------------------------------------------------------- + + +def _closing_section(brief: str) -> list[str]: + """The ``- `` lines directly under the brief's first section.""" + lines = brief.splitlines() + assert lines[0] == "### Closing review", brief + items: list[str] = [] + for line in lines[1:]: + if line.startswith("### ") or line.startswith("## "): + break + if line.startswith("- "): + items.append(line[2:]) + return items + + +def _plant_end_evidence(monkeypatch, *, due: list[dict], mentions: list[dict]) -> None: + """Fixture rows for the end assessment's history readers (the same seam + ``test_now_plan_guidance.py`` uses): no sessions database is involved.""" + from studyloop import history + + monkeypatch.setattr(history, "spaced_repetition_due", lambda topic_keywords_map: list(due)) + monkeypatch.setattr(history.progress, "get_struggling_topics", lambda days=30: []) + monkeypatch.setattr(history, "topic_frequency", lambda keywords, days=90: list(mentions)) + monkeypatch.setattr(history, "last_studied", lambda keywords: None) + monkeypatch.setattr(history, "struggle_topics", lambda days=14, min_sessions=2: []) + + +def test_plan_close_launches_the_architect_with_the_assessment_in_the_brief( + runner, isolated_plans_dir, tmp_path, monkeypatch +) -> None: + """D-G: ``plan close `` on a fully-checked plan is the architect launch — + the one ``study --mode plan-architect`` chain, sibling of ``plan repair`` — + with a brief whose first section is the closing review: the three counts, + the proposal and the evidence lines, readable off the top. The assessment + is the preview: the command writes nothing — document, status and + checkpoint log are untouched — because the learner, not the engine, + decides whether the plan is complete. The due fixture carries a + scheduler "new topic" row (``concept: None``) beside the real due row: + the brief's count is the completion review's — one, not two.""" + from contextlib import ExitStack + + store.plans_dir() + runner.invoke(cli, ["plan", "new", "--title", "Glue ETL", *READY, "--activate"]) + runner.invoke(cli, ["plan", "milestone", "glue-etl", "0", "--done"]) + runner.invoke(cli, ["plan", "milestone", "glue-etl", "1", "--done"]) + assert store.load_plan("glue-etl").milestone_done == 2 # the fixture is fully checked + before = _documents(isolated_plans_dir) + _plant_end_evidence( + monkeypatch, + due=[ + { + "topic": "data-engineering", + "concept": "glue job", + "confidence": "learning", + "last_studied": "2026-09-07", + "days_ago": 9, + "review_type": "overdue", + }, + { + "topic": "data-engineering", + "concept": None, + "confidence": None, + "last_studied": None, + "days_ago": None, + "review_type": "New topic -- start fresh", + "evidence": "configured_topic", + }, + ], + mentions=[{"snippet": "walked through a dynamicframe transform"}], + ) + + captured: dict = {} + calls: list = [] + with ExitStack() as stack: + for p in _launch_patches(tmp_path, captured, calls): + stack.enter_context(p) + monkeypatch.setenv("TMUX", "/tmp/tmux") + result = runner.invoke(cli, ["plan", "close", "glue-etl"]) + + assert result.exit_code == 0, result.output + assert calls == ["Glue ETL"], calls # one launch, topic = the plan's title + assert captured["mode"] == "plan-architect" + + items = _closing_section(captured["brief"]) + assert items[:4] == [ + "Due reviews on plan concepts: 1", + "Struggles on plan concepts: 0", + "Unverified milestones: 0", + "Proposal: extend", + ], items + assert any("glue job" in item for item in items[4:]), items # the evidence names the concept + assert "2/2" in captured["brief"] # milestones done/total, as the plan stands + + intro = captured["brief_intro"] + assert "CLOSING REVIEW" in intro + assert "build a study plan" not in intro + assert "only when the learner agrees" in intro + + assert _documents(isolated_plans_dir) == before + assert store.load_plan("glue-etl").status == "active" + assert index_module.checkpoint_history("glue-etl") == [] + + +def test_plan_close_on_an_unfinished_plan_refuses( + runner, isolated_plans_dir, tmp_path, monkeypatch +) -> None: + """A plan with open milestones has nothing to close: exit 1, the count of + open milestones in the message, no launch, nothing written.""" + from contextlib import ExitStack + + store.plans_dir() + runner.invoke(cli, ["plan", "new", "--title", "Glue ETL", *READY, "--activate"]) + before = _documents(isolated_plans_dir) + + calls: list = [] + with ExitStack() as stack: + for p in _launch_patches(tmp_path, {}, calls): + stack.enter_context(p) + monkeypatch.setenv("TMUX", "/tmp/tmux") + result = runner.invoke(cli, ["plan", "close", "glue-etl"]) + + assert result.exit_code == 1, result.output + clean = _ANSI.sub("", result.output) + assert "'glue-etl' still has 2 open milestone(s)" in clean + assert "Traceback" not in clean + assert calls == [] # no launch + assert _documents(isolated_plans_dir) == before diff --git a/packages/studyloop/tests/test_now_plan_guidance.py b/packages/studyloop/tests/test_now_plan_guidance.py index 20b6fc858..b7a3da01c 100644 --- a/packages/studyloop/tests/test_now_plan_guidance.py +++ b/packages/studyloop/tests/test_now_plan_guidance.py @@ -840,3 +840,215 @@ def test_cli_recap_rich_panel_without_plans_prints_no_plan_line(monkeypatch) -> assert result.exit_code == 0, result.output or repr(result.exception) assert "Plan:" not in result.output assert "Next:" in result.output + + +# --------------------------------------------------------------------------- +# Item 4 (D-G) — evidence-based, consensual completion: rule 9's completion +# action carries the end assessment and proposes; it never changes a status. +# --------------------------------------------------------------------------- + + +def _pre_change_sentence(title: str) -> str: + """The completion sentence rule 9 emitted before D-G (``planning/views.py``).""" + return ( + f"Every milestone of {title!r} is checked off — close the plan " + "or extend it with a follow-on mission." + ) + + +def _plant_evidence( + monkeypatch: pytest.MonkeyPatch, + *, + due: list[dict] | None = None, + struggles: list[dict] | None = None, + mentions: list[dict] | None = None, +) -> None: + """Point the end assessment's history readers at fixture rows. + + ``planning/evaluation.py`` resolves them on the ``studyloop.history`` + package at call time, so the package attribute is the real seam: the + evaluation's own relevance filter and ``has_evidence`` logic stay live, + and nothing here depends on a sessions database. + """ + from studyloop import history + + monkeypatch.setattr( + history, "spaced_repetition_due", lambda topic_keywords_map: list(due or []) + ) + monkeypatch.setattr( + history.progress, "get_struggling_topics", lambda days=30: list(struggles or []) + ) + monkeypatch.setattr(history, "topic_frequency", lambda keywords, days=90: list(mentions or [])) + monkeypatch.setattr(history, "last_studied", lambda keywords: None) + monkeypatch.setattr(history, "struggle_topics", lambda days=14, min_sessions=2: []) + + +_DONE = [ + Milestone(title="A", done=True, concepts=["alpha"]), + Milestone(title="B", done=True, concepts=["beta"]), +] + + +def test_completion_action_carries_the_end_assessment_and_proposes_extend_when_concepts_are_due( + monkeypatch, +) -> None: + """D-G: the completion action carries the end assessment — counts of due + reviews, struggles and unverified milestones on the plan's own concepts — + and proposes ``extend`` while any count is above zero. One due review on + a plan concept is outstanding work: the engine proposes extending, the + evidence names the concept, and the sentence is composed from the + proposal rather than the old either-way wording.""" + _plan("done-plan", title="Done Plan", topics=["sql"], milestones=_DONE) + _plant_evidence( + monkeypatch, + due=[ + { + "topic": "sql", + "concept": "alpha", + "confidence": "learning", + "last_studied": "2026-09-07", + "days_ago": 9, + "review_type": "overdue", + } + ], + mentions=[{"snippet": "worked through beta with a window frame"}], + ) + _patch_collectors(monkeypatch, _candidate("decorators", topic="python", score=100)) + + plan = build_now_plan() + + [action] = plan.completion_actions + assert action.plan_id == "done-plan" + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (1, 0, 0) # pyright: ignore[reportAttributeAccessIssue] + assert action.proposal == "extend" # pyright: ignore[reportAttributeAccessIssue] + assert any("alpha" in line for line in action.evidence), action.evidence # pyright: ignore[reportAttributeAccessIssue] + assert "Done Plan" in action.action + assert "extend" in action.action.lower() + assert action.action != _pre_change_sentence("Done Plan") + + row = plan.to_json_dict()["completion_actions"][0] + assert {"due_reviews", "struggles", "unverified_milestones", "proposal", "evidence"} <= set(row) + assert (row["proposal"], row["due_reviews"]) == ("extend", 1) + assert plan.primary.concept == "decorators" # rule 9 still yields no study candidate + + +def test_completion_action_proposes_close_when_the_assessment_is_clean(monkeypatch) -> None: + """Nothing due, nothing struggling, every checked milestone backed by + evidence: the engine proposes ``close`` — and only proposes (see + :func:`test_completion_never_changes_status`).""" + _plan("done-plan", title="Done Plan", topics=["sql"], milestones=_DONE) + _plant_evidence( + monkeypatch, + mentions=[{"snippet": "explained alpha and beta in the teach-back"}], + ) + _patch_collectors(monkeypatch, _candidate("decorators", topic="python", score=100)) + + plan = build_now_plan() + + [action] = plan.completion_actions + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) # pyright: ignore[reportAttributeAccessIssue] + assert action.proposal == "close" # pyright: ignore[reportAttributeAccessIssue] + assert action.evidence == () # pyright: ignore[reportAttributeAccessIssue] + assert "Done Plan" in action.action + assert "close" in action.action.lower() + assert action.action != _pre_change_sentence("Done Plan") + assert plan.to_json_dict()["completion_actions"][0]["proposal"] == "close" + + +def test_completion_review_does_not_count_new_topic_rows_as_due(monkeypatch) -> None: + """The scheduler's cold-start hint — a ``New topic -- start fresh`` row for + a plan topic with no progress rows, ``concept: None`` — is not a lapsed + review. The completion review counts only rows that name a concept, so a + finished plan whose concepts are backed by session evidence reads + ``close``, not "extend — 1 due review: start fresh". The evaluator keeps + the row (``plan evaluate --phase start`` wants it); this is the completion + review's count, not the evaluator's.""" + _plan("done-plan", title="Done Plan", topics=["sql"], milestones=_DONE) + _plant_evidence( + monkeypatch, + due=[ + { + "topic": "sql", + "concept": None, + "confidence": None, + "last_studied": None, + "days_ago": None, + "review_type": "New topic -- start fresh", + "evidence": "configured_topic", + } + ], + mentions=[{"snippet": "explained alpha and beta in the teach-back"}], + ) + _patch_collectors(monkeypatch, _candidate("decorators", topic="python", score=100)) + + plan = build_now_plan() + + [action] = plan.completion_actions + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) # pyright: ignore[reportAttributeAccessIssue] + assert action.proposal == "close" # pyright: ignore[reportAttributeAccessIssue] + assert action.evidence == () # pyright: ignore[reportAttributeAccessIssue] + + +def test_completion_never_changes_status(monkeypatch) -> None: + """#7 / ``NOT_AUTOMATIC``: the assessment is the preview path — exactly one + ``assess`` per fully-checked plan with ``phase="end"`` and + ``record=False`` — so the document's bytes and status are unchanged after + ``build_now_plan``, no checkpoint row is written and the recording writer + is never called. ``set_study_plan_status`` stays the only door to + ``complete``.""" + from studyloop.planning import AssessPlan + from studyloop.planning import evaluation as evaluation_module + from studyloop.planning import index as plan_index + from studyloop.planning.application import PlanApplication + + _plan("done-plan", title="Done Plan", milestones=_DONE) + path = store.plan_path("done-plan") + before = path.read_bytes() + _plant_evidence(monkeypatch) + _patch_collectors(monkeypatch, _candidate("decorators", topic="python", score=100)) + + intents: list[AssessPlan] = [] + real_assess = PlanApplication.assess + + def counted(self, intent): + intents.append(intent) + return real_assess(self, intent) + + def forbidden(*args, **kwargs): + raise AssertionError("the ranker recorded a checkpoint") + + monkeypatch.setattr(PlanApplication, "assess", counted) + monkeypatch.setattr(evaluation_module, "evaluate_and_record", forbidden) + monkeypatch.setattr(plan_index, "record_checkpoint", forbidden) + + plan = build_now_plan() + + assert [action.plan_id for action in plan.completion_actions] == ["done-plan"] + assert [(i.plan_id, i.phase, i.record) for i in intents] == [("done-plan", "end", False)] + assert path.read_bytes() == before + assert store.load_plan("done-plan").status == "active" + assert plan_index.checkpoint_history("done-plan") == [] + + +def test_completion_assessment_failure_keeps_the_sentence_and_warns(monkeypatch) -> None: + """A failed assessment is a warning, never a failed ``now``: the completion + action still appears with the pre-change sentence, and ``warnings`` names + the plan so the learner knows the counts are missing rather than zero.""" + from studyloop.planning.application import PlanApplication + + _plan("done-plan", title="Done Plan", milestones=_DONE) + _patch_collectors(monkeypatch, _candidate("decorators", topic="python", score=100)) + + def boom(self, intent): + raise RuntimeError("sessions.db is locked") + + monkeypatch.setattr(PlanApplication, "assess", boom) + + plan = build_now_plan() + + [action] = plan.completion_actions + assert action.action == _pre_change_sentence("Done Plan") + assert any( + "done-plan" in warning and "assess" in warning.lower() for warning in plan.warnings + ), plan.warnings + assert plan.primary.concept == "decorators" From f1c52ce8a3e30d13d7ac7778f3f47ef6a9de6341 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Thu, 17 Sep 2026 19:23:44 +0100 Subject: [PATCH 02/23] docs(plan-integration): tick T4.1 with its receipt; record what the item-4 RED pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design §4 now states the closing-review brief format, intro and refusal text the RED fixed; records the owner's 2026-09-17 decision that the completion review counts only due rows naming a concept (the scheduler's "new topic" row is excluded, with the reasoning and the measured cost of the preview); and keeps the one decision deliberately left open — what `proposal` holds when the end assessment fails — with the default GREEN will take unless vetoed (nullable proposal, counts stay int). --- .../plan-integration-followons/design.md | 22 +++++++++++++++++++ .../plan-integration-followons/tasks.md | 8 ++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/openspec/changes/plan-integration-followons/design.md b/openspec/changes/plan-integration-followons/design.md index 344cb007c..98b1fc6f0 100644 --- a/openspec/changes/plan-integration-followons/design.md +++ b/openspec/changes/plan-integration-followons/design.md @@ -158,6 +158,28 @@ among what MCP revises and the row names every schema property. no-plan golden is unchanged. - **Persona:** an "Extend or close" subsection: read the evidence back; propose; ask "anything you are not comfortable with?"; change status only when the learner agrees. +- **Pinned by the RED (`14c8938b`, 2026-09-17):** the `### Closing review` section's first four `- ` lines are + `Due reviews on plan concepts: N`, `Struggles on plan concepts: N`, `Unverified milestones: N`, + `Proposal: extend|close`, followed by the evidence lines — readable off the top, as the repair section's + blockers are. The intro says `CLOSING REVIEW`, does not say "build a study plan", and says the status changes + "only when the learner agrees". The composed sentence differs from the pre-change either-way sentence and + names the proposal. The refusal is `'' still has N open milestone(s)` (exit 1, no launch). +- **Decided by the owner (2026-09-17), pinned by the RED:** the completion review counts only due rows that + name a concept. `spaced_repetition_due` appends a `New topic -- start fresh` row (`concept: None`, + `evidence: configured_topic`) for every plan topic with no progress rows — the scheduler's cold-start hint + for "what should I review now", not a lapsed review. The evaluator already ignores it at concept level (a + `None` concept never matches a milestone concept, so it contributes nothing to `unverified_milestones`), and + counting it would tell a learner who has just ticked every milestone to "start fresh" — the + incompleteness-after-success framing D-G exists to avoid. `unverified_milestones` remains the honest carrier + of "done without evidence". `plan evaluate` keeps the row (phase `start` wants it); the exclusion is the + completion review's, one definition beside `PlanEvaluationView` in `planning/views.py`, consumed by both the + engine's `CompletionAction` and the `plan close` brief. Measured cost of the preview on the live 877 MB + database: ~320 ms per fully-checked plan per `build_now_plan` (five readers), a transient state by design. +- **Open for GREEN (not pinned):** what `proposal` holds when the assessment fails. `Literal["extend", "close"]` + has no honest value for "not assessed" — `extend` asserts outstanding work without evidence, `close` asserts + a clean slate without evidence. Default unless vetoed: `proposal: Literal["extend", "close"] | None`, `None` + on failure with the counts `0` and `evidence` empty; the `warnings` entry explains; renderers print the plain + sentence when `proposal is None`. One nullable field carries the state; the three counts keep their type. ## 5. Item 5 — per-item energy demand and the body-doubling floor (D-F) — designed here, reviewed separately diff --git a/openspec/changes/plan-integration-followons/tasks.md b/openspec/changes/plan-integration-followons/tasks.md index 3909c0cae..dbc0412fd 100644 --- a/openspec/changes/plan-integration-followons/tasks.md +++ b/openspec/changes/plan-integration-followons/tasks.md @@ -116,7 +116,13 @@ writer, through the existing gate, closes that. Kept out of item 3 so item 3's f ## Item 4 — `plan close ` (D-G) · files: `learning/decision.py`, `cli/{_plan,_now}.py`, `learning/recap.py`, `web/static/js/components/today-panel.js`, `web/static/index.html`, persona (+ projections + manifest), tests, `docs/study-plans.md`, spec delta `active-learning-decisions`, `cli-surface` -- [ ] **T4.1** RED `tests/test_now_plan_guidance.py`: +- [x] **T4.1** (RED `14c8938b`: 7 failed / 57 passed across the two files, each on the intended reason — missing + `CompletionAction` attributes, `assess` never called, no warning, no `close` command; ruff + pyright + clean; hooks first time; golden sha unchanged. Seventh test, added on the owner's 2026-09-17 decision: + `test_completion_review_does_not_count_new_topic_rows_as_due`; the seam launch test's due fixture carries + a new-topic row beside the real one and still asserts a count of 1. Landed name of the first test drops + one redundant word: `…_proposes_extend_when_concepts_are_due` — no def line in the repo exceeds 100 + chars.) RED `tests/test_now_plan_guidance.py`: `test_completion_action_carries_the_end_assessment_and_proposes_extend_when_plan_concepts_are_due`, `test_completion_action_proposes_close_when_the_assessment_is_clean`, `test_completion_never_changes_status` (document bytes and status unchanged after `build_now_plan`; no checkpoint row written), From 82293293d6e4e343f6e83eb831fea5eb29c7e2b8 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 09:14:54 +0100 Subject: [PATCH 03/23] =?UTF-8?q?feat(plan):=20GREEN=20for=20item=204=20?= =?UTF-8?q?=E2=80=94=20evidence-based,=20consensual=20completion=20(D-G)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fully-checked active plan used to get an either-way sentence ("close the plan or extend it") that the owner scored "no as phrased" on rubric row 4: it proposed nothing and rested on nothing. The completion action is now a closing review read from the plan's own end assessment, and `plan close ` is the door to acting on it — with the learner, never automatically. One definition, two surfaces. `planning.views.CompletionReview` reads the `end` evaluation into three counts on the plan's concepts (due reviews, struggles, milestones marked done with no evidence), a proposal (`extend` while any count is above zero, else `close`) and one evidence line per counted item (capped at 8). Both the `now` engine's `CompletionAction` and the `plan close` brief consume it, so they cannot disagree on a count. Due reviews count only rows that name a concept (owner decision, 2026-09-17): the scheduler's `New topic -- start fresh` row is a cold-start hint for "what should I review now", not a lapsed review, and counting it would tell a learner who just ticked every milestone to start fresh. `plan evaluate` keeps the row; the exclusion is the completion review's. The engine reads through the preview path — `assess(AssessPlan(phase="end", record=False))`, one call per fully-checked plan per `build_now_plan` — so the document, its status and the checkpoint log are untouched. A failed assessment keeps the plain sentence with `proposal` None and the counts 0, plus a warning naming the plan: the counts are then unknown, not zero, and no renderer reads a clean slate or outstanding work into a failure. Measured on the live 877 MB sessions.db the preview costs ~320 ms per fully-checked plan, a transient state by design. New JSON keys appear only inside `completion_actions` entries; the no-plan golden is byte-identical. `plan close ` mirrors `plan repair`: `_inspect`, refuse with the open count (exit 1) while any milestone is open, leave a `complete` plan alone, and for a fully-checked plan launch the architect through the one `study --mode plan-architect` chain with `### Closing review` as the brief's first section (the four count/proposal lines, then the evidence). The shared "plan as it stands" section is refactored out of the repair brief byte-identically. The persona gains "Extend or close" (three projections re-projected, manifest regenerated with `updated` restored on unmoved entries, secrets baseline refreshed whole-repo: 72 -> 72 files, exactly the two manifest hashes). CLI `now` prints the evidence lines beneath the sentence; the Today card gains `completionEvidence()` for the same lines (JS 136/136); the recap speaks the composed sentence. Docs, two spec deltas, and rubric row 4b (re-run primary and both proposals, verdict PENDING for the owner; row 4 kept as the record of the finding). Verification: 7 RED -> green (64/64 across the two files); golden sha ec451ce8 unchanged; `just lint`, `just typecheck`, `mkdocs build --strict`, `openspec validate` clean. Full suite 30 failed / 7233 passed / 14 errors, diffed against a clean f1c52ce8 control run in parallel: item4 - control = empty, control - item4 = exactly the seven REDs, 44 shared environmental ids committed by name in receipts/full-suite-control-item4-2026-09-18.md. --- .secrets.baseline | 6 +- agents/claude/study-plan-architect.md | 34 +++++ agents/kiro/study-plan-architect/persona.md | 34 +++++ agents/manifest.json | 4 +- agents/opencode/study-plan-architect.md | 34 +++++ agents/shared/personas/plan-architect.md | 34 +++++ .../full-suite-control-item4-2026-09-18.md | 74 ++++++++++ .../receipts/now-rubric-2026-09-16.md | 9 +- docs/cli-reference.md | 2 + docs/study-plans.md | 26 +++- .../specs/active-learning-decisions/spec.md | 74 ++++++++++ .../specs/cli-surface/spec.md | 46 ++++++ packages/studyloop/src/studyloop/cli/_now.py | 4 + packages/studyloop/src/studyloop/cli/_plan.py | 133 ++++++++++++++++-- .../src/studyloop/learning/decision.py | 128 +++++++++++++++-- .../src/studyloop/planning/__init__.py | 4 + .../studyloop/src/studyloop/planning/views.py | 82 +++++++++++ .../src/studyloop/web/static/index.html | 3 + .../web/static/js/components/today-panel.js | 10 ++ .../src/studyloop/web/static/style.css | 2 + .../tests/js/today-panel-plan.test.js | 33 +++++ .../studyloop/tests/test_now_plan_guidance.py | 18 +-- 22 files changed, 753 insertions(+), 41 deletions(-) create mode 100644 docs/architecture/plan-integration/receipts/full-suite-control-item4-2026-09-18.md create mode 100644 openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md diff --git a/.secrets.baseline b/.secrets.baseline index e9c1b887e..4d5cafbde 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -144,7 +144,7 @@ { "type": "Hex High Entropy String", "filename": "agents/manifest.json", - "hashed_secret": "f8b90f6828d80715ff01f752fb0e47bb26ece8b7", + "hashed_secret": "1953bb2b37b175c55c56751ab15fdaa32524b144", "is_verified": false, "line_number": 9 }, @@ -179,7 +179,7 @@ { "type": "Hex High Entropy String", "filename": "agents/manifest.json", - "hashed_secret": "d7bd0c449bbfc17c19acc39f4cac943e5f9f8a2f", + "hashed_secret": "6a33435d0d8b33083d2d7689d9d6c6a8c8b4bc6b", "is_verified": false, "line_number": 33 }, @@ -2056,5 +2056,5 @@ } ] }, - "generated_at": "2026-09-17T10:27:53Z" + "generated_at": "2026-09-17T21:00:18Z" } diff --git a/agents/claude/study-plan-architect.md b/agents/claude/study-plan-architect.md index c67a62977..86e45dae8 100644 --- a/agents/claude/study-plan-architect.md +++ b/agents/claude/study-plan-architect.md @@ -196,6 +196,40 @@ leave the edit to the learner (the `## Mission` and `## Milestones` sections of the document, or the Web UI's plan editor), then `studyloop plan show PLAN_ID --json` to read `readiness` back. +## Closing a Plan + +A plan whose every milestone is checked is finished work, not yet a finished +plan. `studyloop now` and the Today card report it as a completion action that +carries the end assessment on the plan's own concepts — due reviews, struggles, +and milestones marked done without evidence — and a proposal: `extend` while any +count is above zero, `close` when all three are zero. `studyloop plan close +PLAN_ID` launches you with a brief whose first section, **Closing review**, +lists the three counts, the proposal and one line per counted item, followed by +the plan as it stands. The brief's opening line says this is a CLOSING REVIEW +session. The review counts only due rows that name a concept: the scheduler's +"New topic -- start fresh" hint is not outstanding work. Then: + +1. Read the evidence back, line by line, before you say what you think. The + counts are the databases' view; the learner's view is the one that decides. +2. Propose — extend or close — and say why in one sentence, from the evidence. + Extending means a follow-on mission for what is still due or unverified, + never re-opening a ticked milestone; closing means `complete`. +3. Ask: "Is there anything here you are not comfortable with?" Then wait. +4. Change the status only when the learner agrees, and only to what they + agreed. To close: `set_study_plan_status(plan_id, "complete")` (fallback: + `studyloop plan status PLAN_ID complete`). To extend: revise the plan with + `update_study_plan` — new milestones on the outstanding work, or a follow-on + plan through the interview — and leave it `active`. Never change a status + because the proposal said so: the engine proposes, you ask, the learner + decides. +5. Before closing, offer to record what was learned (`record_plan_learning`, + the wind-down's first write) and to log confidence on any concept that was + never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the + spaced-repetition loop keeps what the plan taught. + +If the brief carries a **Data gaps** section, the counts are partial. Say so +before you propose anything. + ## Evaluating a Plan | Phase | When | Question it answers | diff --git a/agents/kiro/study-plan-architect/persona.md b/agents/kiro/study-plan-architect/persona.md index 6636e4b22..ecdf633f4 100644 --- a/agents/kiro/study-plan-architect/persona.md +++ b/agents/kiro/study-plan-architect/persona.md @@ -190,6 +190,40 @@ leave the edit to the learner (the `## Mission` and `## Milestones` sections of the document, or the Web UI's plan editor), then `studyloop plan show PLAN_ID --json` to read `readiness` back. +## Closing a Plan + +A plan whose every milestone is checked is finished work, not yet a finished +plan. `studyloop now` and the Today card report it as a completion action that +carries the end assessment on the plan's own concepts — due reviews, struggles, +and milestones marked done without evidence — and a proposal: `extend` while any +count is above zero, `close` when all three are zero. `studyloop plan close +PLAN_ID` launches you with a brief whose first section, **Closing review**, +lists the three counts, the proposal and one line per counted item, followed by +the plan as it stands. The brief's opening line says this is a CLOSING REVIEW +session. The review counts only due rows that name a concept: the scheduler's +"New topic -- start fresh" hint is not outstanding work. Then: + +1. Read the evidence back, line by line, before you say what you think. The + counts are the databases' view; the learner's view is the one that decides. +2. Propose — extend or close — and say why in one sentence, from the evidence. + Extending means a follow-on mission for what is still due or unverified, + never re-opening a ticked milestone; closing means `complete`. +3. Ask: "Is there anything here you are not comfortable with?" Then wait. +4. Change the status only when the learner agrees, and only to what they + agreed. To close: `set_study_plan_status(plan_id, "complete")` (fallback: + `studyloop plan status PLAN_ID complete`). To extend: revise the plan with + `update_study_plan` — new milestones on the outstanding work, or a follow-on + plan through the interview — and leave it `active`. Never change a status + because the proposal said so: the engine proposes, you ask, the learner + decides. +5. Before closing, offer to record what was learned (`record_plan_learning`, + the wind-down's first write) and to log confidence on any concept that was + never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the + spaced-repetition loop keeps what the plan taught. + +If the brief carries a **Data gaps** section, the counts are partial. Say so +before you propose anything. + ## Evaluating a Plan | Phase | When | Question it answers | diff --git a/agents/manifest.json b/agents/manifest.json index 0b1c8db3c..f10e39254 100644 --- a/agents/manifest.json +++ b/agents/manifest.json @@ -6,7 +6,7 @@ "updated": "2026-09-14" }, "claude/study-plan-architect.md": { - "hash": "a98e605e8e0db101", + "hash": "30931bca52b881ac", "updated": "2026-09-17" }, "codex/AGENTS.md": { @@ -30,7 +30,7 @@ "updated": "2026-09-14" }, "opencode/study-plan-architect.md": { - "hash": "f9af2487053cbc65", + "hash": "54a0b303285cbad6", "updated": "2026-09-17" }, "pi/AGENTS.md": { diff --git a/agents/opencode/study-plan-architect.md b/agents/opencode/study-plan-architect.md index 298531aa7..11bcbbb4c 100644 --- a/agents/opencode/study-plan-architect.md +++ b/agents/opencode/study-plan-architect.md @@ -207,6 +207,40 @@ leave the edit to the learner (the `## Mission` and `## Milestones` sections of the document, or the Web UI's plan editor), then `studyloop plan show PLAN_ID --json` to read `readiness` back. +## Closing a Plan + +A plan whose every milestone is checked is finished work, not yet a finished +plan. `studyloop now` and the Today card report it as a completion action that +carries the end assessment on the plan's own concepts — due reviews, struggles, +and milestones marked done without evidence — and a proposal: `extend` while any +count is above zero, `close` when all three are zero. `studyloop plan close +PLAN_ID` launches you with a brief whose first section, **Closing review**, +lists the three counts, the proposal and one line per counted item, followed by +the plan as it stands. The brief's opening line says this is a CLOSING REVIEW +session. The review counts only due rows that name a concept: the scheduler's +"New topic -- start fresh" hint is not outstanding work. Then: + +1. Read the evidence back, line by line, before you say what you think. The + counts are the databases' view; the learner's view is the one that decides. +2. Propose — extend or close — and say why in one sentence, from the evidence. + Extending means a follow-on mission for what is still due or unverified, + never re-opening a ticked milestone; closing means `complete`. +3. Ask: "Is there anything here you are not comfortable with?" Then wait. +4. Change the status only when the learner agrees, and only to what they + agreed. To close: `set_study_plan_status(plan_id, "complete")` (fallback: + `studyloop plan status PLAN_ID complete`). To extend: revise the plan with + `update_study_plan` — new milestones on the outstanding work, or a follow-on + plan through the interview — and leave it `active`. Never change a status + because the proposal said so: the engine proposes, you ask, the learner + decides. +5. Before closing, offer to record what was learned (`record_plan_learning`, + the wind-down's first write) and to log confidence on any concept that was + never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the + spaced-repetition loop keeps what the plan taught. + +If the brief carries a **Data gaps** section, the counts are partial. Say so +before you propose anything. + ## Evaluating a Plan | Phase | When | Question it answers | diff --git a/agents/shared/personas/plan-architect.md b/agents/shared/personas/plan-architect.md index 6636e4b22..ecdf633f4 100644 --- a/agents/shared/personas/plan-architect.md +++ b/agents/shared/personas/plan-architect.md @@ -190,6 +190,40 @@ leave the edit to the learner (the `## Mission` and `## Milestones` sections of the document, or the Web UI's plan editor), then `studyloop plan show PLAN_ID --json` to read `readiness` back. +## Closing a Plan + +A plan whose every milestone is checked is finished work, not yet a finished +plan. `studyloop now` and the Today card report it as a completion action that +carries the end assessment on the plan's own concepts — due reviews, struggles, +and milestones marked done without evidence — and a proposal: `extend` while any +count is above zero, `close` when all three are zero. `studyloop plan close +PLAN_ID` launches you with a brief whose first section, **Closing review**, +lists the three counts, the proposal and one line per counted item, followed by +the plan as it stands. The brief's opening line says this is a CLOSING REVIEW +session. The review counts only due rows that name a concept: the scheduler's +"New topic -- start fresh" hint is not outstanding work. Then: + +1. Read the evidence back, line by line, before you say what you think. The + counts are the databases' view; the learner's view is the one that decides. +2. Propose — extend or close — and say why in one sentence, from the evidence. + Extending means a follow-on mission for what is still due or unverified, + never re-opening a ticked milestone; closing means `complete`. +3. Ask: "Is there anything here you are not comfortable with?" Then wait. +4. Change the status only when the learner agrees, and only to what they + agreed. To close: `set_study_plan_status(plan_id, "complete")` (fallback: + `studyloop plan status PLAN_ID complete`). To extend: revise the plan with + `update_study_plan` — new milestones on the outstanding work, or a follow-on + plan through the interview — and leave it `active`. Never change a status + because the proposal said so: the engine proposes, you ask, the learner + decides. +5. Before closing, offer to record what was learned (`record_plan_learning`, + the wind-down's first write) and to log confidence on any concept that was + never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the + spaced-repetition loop keeps what the plan taught. + +If the brief carries a **Data gaps** section, the counts are partial. Say so +before you propose anything. + ## Evaluating a Plan | Phase | When | Question it answers | diff --git a/docs/architecture/plan-integration/receipts/full-suite-control-item4-2026-09-18.md b/docs/architecture/plan-integration/receipts/full-suite-control-item4-2026-09-18.md new file mode 100644 index 000000000..75e0aa1c4 --- /dev/null +++ b/docs/architecture/plan-integration/receipts/full-suite-control-item4-2026-09-18.md @@ -0,0 +1,74 @@ +# Full suite, matched control — item 4 GREEN · 2026-09-18 + +Two full `pytest` runs in parallel on this host (macOS sandbox), same command: +`uv run --group dev pytest -q -p no:cacheprovider -rfE`. + +- **item 4 tree** (working tree on `feat/plan-close`, GREEN uncommitted at run time): 30 failed / 7233 passed / 4 skipped / 14 errors (952 s). +- **control** (clean worktree at the RED tip `f1c52ce8`, own `uv sync --group dev --all-packages`): 37 failed / 7191 passed / 16 skipped / 14 errors (956 s). + +Sorted failure+error id sets, diffed: + +- item4 − control = **∅** (zero regressions). +- control − item4 = exactly the seven item-4 RED tests (red on the control tip by construction). +- shared: **44** ids — the sandbox-environmental class (journey world guards, acceptance isolation, harness-matrix live mechanics, brain CLI, doctor second-brain vault, one agent-session-tools eval arm). Items 3 and 3b recorded 45 shared ids, but that list was never persisted (session scratch), so which id differs cannot be named here; what this run proves is only that the two trees fail on the same 44 and differ on exactly the seven REDs. The list below is committed so the next item can diff against it by name. + +## Shared environmental ids + +``` +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_a_journey_transcript_records_every_command_and_its_output +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_every_world_path_lives_under_the_temp_root +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_redaction_leaves_the_vault_relative_paths_a_reader_needs +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_the_cli_runs_inside_the_world_not_the_host +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_the_environment_handed_to_the_child_names_no_real_directory +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_the_transcript_carries_no_username_or_home_path +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_the_week_world_cannot_resolve_the_personal_vault +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_the_week_world_cannot_resolve_the_real_config_dir +packages/studyloop/tests/journeys/test_journey_world_guards.py::test_the_week_world_starts_with_no_provider +packages/studyloop/tests/journeys/test_obsidian_learners_week.py::test_a_learners_week_in_order +packages/studyloop/tests/journeys/test_xtiles_learners_week.py::test_a_study_day_when_the_provider_cannot_publish +packages/studyloop/tests/journeys/test_xtiles_learners_week.py::test_the_canary_check_can_actually_fail +packages/studyloop/tests/journeys/test_xtiles_learners_week.py::test_the_xtiles_week_stores_no_credential +packages/studyloop/tests/journeys/test_xtiles_prompt_inputs.py::test_the_project_prompt_input_is_producible +packages/studyloop/tests/test_acceptance_isolation.py::TestRealHarnessAuthMode::test_default_mode_is_unchanged_and_records_itself +packages/studyloop/tests/test_acceptance_isolation.py::TestRealHarnessAuthMode::test_harness_home_is_real_but_every_studyloop_pointer_is_scratch +packages/studyloop/tests/test_acceptance_isolation.py::TestRealHarnessAuthMode::test_sweep_removes_the_tmux_socket_dir_even_though_it_is_outside_home +packages/studyloop/tests/test_acceptance_isolation.py::TestRealHarnessAuthMode::test_sweep_still_never_touches_the_real_home +packages/studyloop/tests/test_acceptance_isolation.py::TestScratchEnvironmentContextManager::test_swept_even_when_the_body_raises +packages/studyloop/tests/test_acceptance_isolation.py::TestScratchTmuxSocketDirIsUsable::test_a_real_tmux_session_starts_under_the_scratch_socket_dir +packages/studyloop/tests/test_acceptance_isolation.py::TestSweepGuards::test_normal_scratch_sweeps_cleanly +packages/studyloop/tests/test_acceptance_isolation.py::TestTmuxDescendantStopper::test_sweep_kills_the_scratch_tmux_server_first +packages/studyloop/tests/test_cli_brain.py::test_dry_run_reports_a_refusal_it_would_actually_hit +packages/studyloop/tests/test_cli_brain.py::test_enable_prints_the_resolved_vault +packages/studyloop/tests/test_cli_brain.py::test_publish_missing_vault_exit_1_nothing_written +packages/studyloop/tests/test_cli_brain.py::test_pull_prints_notes +packages/studyloop/tests/test_cli_brain.py::test_template_install_creates_only +packages/studyloop/tests/test_cli_brain.py::test_template_install_is_all_or_nothing +packages/studyloop/tests/test_cli_brain.py::test_template_install_refuses_existing +packages/studyloop/tests/test_config_init_second_brain.py::test_what_is_written_loads_back_cleanly +packages/studyloop/tests/test_doctor_second_brain.py::test_rows_vault_missing_warns +packages/studyloop/tests/test_fresh_install_scope.py::test_studyloop_study_exits_2_with_the_diagnostic_on_a_virgin_home +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_real_auth_scratch_records_real_auth_for_every_harness[claude] +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_real_auth_scratch_records_real_auth_for_every_harness[codex] +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_real_auth_scratch_records_real_auth_for_every_harness[grok] +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_real_auth_scratch_records_real_auth_for_every_harness[kiro] +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_real_auth_scratch_records_real_auth_for_every_harness[opencode] +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_real_auth_scratch_records_real_auth_for_every_harness[pi] +packages/studyloop/tests/test_harness_matrix_live_mechanics.py::TestAuthModeRecording::test_scrubbed_scratch_keeps_the_original_split +packages/studyloop/tests/test_obsidian_vault_isolation.py::test_an_explicit_configured_vault_still_wins_over_the_override +packages/studyloop/tests/test_obsidian_vault_isolation.py::test_real_default_vault_is_unreachable +packages/studyloop/tests/test_obsidian_vault_isolation.py::test_the_isolation_override_is_set_for_every_test +packages/studyloop/tests/test_second_brain_cli_core.py::test_status_json_obsidian_shape +packages/studyloop/tests/test_second_brain_cli_core.py::test_status_reports_a_missing_vault_without_failing +``` + +## Only on the control (the REDs) + +``` +packages/studyloop/tests/test_cli_plan_seam.py::test_plan_close_launches_the_architect_with_the_assessment_in_the_brief +packages/studyloop/tests/test_cli_plan_seam.py::test_plan_close_on_an_unfinished_plan_refuses +packages/studyloop/tests/test_now_plan_guidance.py::test_completion_action_carries_the_end_assessment_and_proposes_extend_when_concepts_are_due +packages/studyloop/tests/test_now_plan_guidance.py::test_completion_action_proposes_close_when_the_assessment_is_clean +packages/studyloop/tests/test_now_plan_guidance.py::test_completion_assessment_failure_keeps_the_sentence_and_warns +packages/studyloop/tests/test_now_plan_guidance.py::test_completion_never_changes_status +packages/studyloop/tests/test_now_plan_guidance.py::test_completion_review_does_not_count_new_topic_rows_as_due +``` diff --git a/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md b/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md index ea490b59f..a657c5d5e 100644 --- a/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md +++ b/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md @@ -1,6 +1,6 @@ # Plan-aware `now` — five-scenario human rubric (D-16) · 2026-09-16 -**Status: owner verdicts RECORDED 2026-09-16 (interactive walkthrough with the coordinator).** Scenarios 1, 2 and the primary of 4: yes. Scenario 3: **no** (finding). Scenario 4 completion action: **no as phrased** (finding). Scenario 5: verified. This receipt was produced unattended +**Status: owner verdicts RECORDED 2026-09-16 (interactive walkthrough with the coordinator).** Scenarios 1, 2 and the primary of 4: yes. Scenario 3: **no** (finding). Scenario 4 completion action: **no as phrased** (finding). Scenario 5: verified. **Row 4b added 2026-09-18** (item 4 / D-G, tree `feat/plan-close`): scenario 4 re-run with the end assessment planted, both proposals recorded as emitted; verdict `PENDING` for the owner — row 4 is kept as the record of the original finding. This receipt was produced unattended overnight. Every scenario below was *run* on frozen fixtures and the primary and its rationale are recorded exactly as the engine emitted them; the "would I do the primary?" column is a human judgement that only the owner can @@ -31,6 +31,7 @@ learning"). | 2 | Urgent-unrelated wins | Same plan. One due item: `decorators`/python, base 100 (an overdue spaced-repetition review). Nothing represents milestone 0. | **`decorators`** (118, no refs); alternate `window function` (60, `source=study_plan:sql-windows:0`, `plan_refs=[(sql-windows, 0)]`, reason "Next milestone 1/1 of plan 'SQL Windows': Window basics"). | Rule 5: the unrelated candidate is in a more urgent class (due review) and wins outright — the bias cannot lift new-milestone work over it. Rule 6: the plan's next milestone was unrepresented, so it was synthesised at base 48 + bias 12 = 60 and appears as the plan-backed alternate (rule 8 satisfied without any swap). | **yes** — owner, 2026-09-16: clear the overdue review first. Note for follow-on: an overdue item *unrelated* to the plan must not sit as an alternate indefinitely — propose it explicitly (age-aware nudge) or let the learner retire it. | | 3 | Energy-deferred | Plan `sql-windows` with `energy_floor: 5`; milestone 0 `Window basics` **done** (concepts `[window function]`), milestone 1 `Frames` (concepts `[window frame]`). One struggle-repair item `window function`/sql, `hands-on`, base 82. **Energy `low`** (capability 3/10). | **`window function`** (hands-on, score 80, `plan_refs=[(sql-windows, None)]`); no alternates; `energy_deferred=[(sql-windows, milestone 1, floor 5, capability 3)]`; JSON gains `energy_deferred`. | Rule 3: capability 3 < floor 5, so the *new* milestone (Frames) is deferred and named, not synthesised; the plan-related repair on a finished milestone's concept stays eligible and keeps its ref (`None`: plan-related repair, not the next milestone). Score = 82 + 12 bias − 14 (hands-on at low energy). | **no** — owner, 2026-09-16: a struggle-repair task has no energy demand of its own; recommending hands-on repair of a *live* struggle on a low-energy day risks compounding the struggle and damaging confidence (RSD). Finding for council: (1) derive a per-item energy demand for repair from struggle recency/teach-back — at low energy a live struggle defers like new work, a recovered one stays eligible as gentle review; (2) when nothing plan-related fits the day's capability, synthesise a body-doubling / open-session candidate (feature exists: ADR-0001/0003, `web/routes/body_double.py`) naming the deferred items, instead of the least-bad task. | | 4 | Fully-checked | Plan `done-plan` ("Done Plan"), milestones A and B both done. One due item `decorators`/python base 100. | **`decorators`** (118, no refs); `completion_actions=[(done-plan, "Every milestone of 'Done Plan' is checked off — close the plan or extend it with a follow-on mission.")]`; no `study_plan:` candidate anywhere; JSON gains `active_plans` + `completion_actions`. | Rule 9: a fully-checked plan is reported as a completion action and is neither matched (no bias, no refs) nor synthesised. | **primary yes / completion action no as phrased** — owner, 2026-09-16: the completion action must be contextual and consensual. Run the end assessment (`assess(phase="end")`: due reviews, struggles, unverified milestones on the plan's concepts). If outstanding work touches the plan's concepts (or their prerequisites — F2 concept edges), propose *extend* and name the evidence; if clean, propose *close* and ask the learner to agree ("anything you are not comfortable with?"). Status never changes automatically (#7). Natural vehicle: architect with `purpose=planning` and the assessment in the brief (`plan close `, sibling of `plan repair `). Finding for council. | +| 4b | Fully-checked — **re-run after D-G (item 4, 2026-09-18)** | Row 4's fixture (`done-plan`, milestones A `[alpha]` and B `[beta]` both done; one due item `decorators`/python base 100), plus the end assessment's readers planted: **(a)** one due review on plan concept `alpha` (`overdue`) with session mentions backing both concepts; **(b)** no due rows, same mentions. | Primary unchanged in both: **`decorators`** (118, no refs); no `study_plan:` candidate. **(a)** `completion_actions=[(done-plan, due 1 / struggles 0 / unverified 0, proposal **extend**, evidence `["Due review: alpha — overdue"]`)]`, sentence: "Every milestone of 'Done Plan' is checked off, and the closing review proposes extending the plan — 1 due review, 0 struggles and 0 unverified milestones on its concepts. Walk the evidence with the architect: studyloop plan close done-plan." **(b)** counts 0/0/0, proposal **close**, evidence `[]`, sentence: "Every milestone of 'Done Plan' is checked off and the closing review is clean — it proposes closing the plan. Close it with the architect when you agree: studyloop plan close done-plan." No warnings; JSON gains the five keys only inside the entry. | Rule 9 as before for the ranking. The completion action is now the end assessment read as a preview (`assess(phase="end", record=False)`, one call, no write, no status change): `extend` iff any of the three counts on the plan's own concepts is above zero, else `close`; due counts only rows naming a concept (the scheduler's "new topic" row is excluded — owner decision 2026-09-17). `plan close done-plan` launches the architect with the same review as the brief's first section; status moves only when the learner agrees. | **PENDING** — owner: does (a) read as a proposal you would walk, and (b) as a close you would agree to? | | 5 | No-plan identical | No plan documents at all; no collector candidates. | **`one tiny recall loop`** (python, recall, 28, `source=starter`) — the starter. | D-5: `serialise(plan) == golden` → **byte-identical** (`True` in the run); the JSON key list is exactly the golden's — no additive key is present. | **verified** — owner walkthrough 2026-09-16: nothing to judge; the byte-identical golden is the acceptance. | ## How to re-run @@ -46,7 +47,11 @@ The five rows correspond to `test_fully_checked_active_plan_emits_completion_not_candidate` and `test_no_active_plans_json_byte_identical_to_golden`; the primaries above are what those tests assert, printed from a throwaway driver over the same -fixtures. +fixtures. Row 4b corresponds to +`test_completion_action_carries_the_end_assessment_and_proposes_extend_when_concepts_are_due` +(reading a) and `test_completion_action_proposes_close_when_the_assessment_is_clean` +(reading b), printed the same way on 2026-09-18 with the end assessment's +history readers planted through `_plant_evidence`. ## What the owner should do diff --git a/docs/cli-reference.md b/docs/cli-reference.md index d6886ea88..3a42061b8 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -85,6 +85,7 @@ studyloop plan new --title TITLE [--why WHY] [--topic T] [--success S] [--milest studyloop plan new --title TITLE --activate # Activate on create (refused if incomplete) studyloop plan list [--status draft|active|paused|complete|abandoned] [--husks] [--json] # `!` after the status marks an active plan that is not ready; --json rows carry `ready` studyloop plan repair PLAN_ID [--agent A] # Launch the architect on an active-but-unready plan with its blockers in the brief (writes nothing itself) +studyloop plan close PLAN_ID [--agent A] # Launch the architect on a fully-checked plan with the closing review in the brief; status changes only when you agree studyloop plan show PLAN_ID [--markdown] [--json] studyloop plan status PLAN_ID active # Change lifecycle state studyloop plan milestone PLAN_ID INDEX [--done|--undone] # Toggle or set a milestone @@ -391,6 +392,7 @@ studyloop plan record PLAN_ID --title T [--body B|--body-file F] [--status S] [- studyloop plan reindex # Rebuild the DB index from the documents studyloop plan architect [--agent claude] # Launch the study-plan-architect (studyloop study --mode plan-architect) studyloop plan repair PLAN_ID # Same launch chain, briefed with the blockers of an active plan that is not ready +studyloop plan close PLAN_ID # Same launch chain, briefed with the closing review of a plan whose every milestone is checked ``` Omitted answers are left **explicitly blank** in the document rather than invented, and `readiness` reports what is still missing. Activation (`--activate`, or `plan status … active`) is **refused** while a plan lacks a mission, success criteria, or milestones — an unevaluable plan must not look active. diff --git a/docs/study-plans.md b/docs/study-plans.md index 0a16fbf5f..e08902218 100644 --- a/docs/study-plans.md +++ b/docs/study-plans.md @@ -224,7 +224,20 @@ is **not ready** — a hand edit removed its mission or its milestones — is listed with a warning naming what to repair; it still biases related work, but no milestone is suggested for it until it is paused or repaired. A plan whose milestones are all checked appears as a completion action instead of -new work. This is plan-aware guidance with tested ranking rules — a bias, not +new work, and that action is a **closing review**, not a verdict: the engine +reads the plan's end assessment as a preview — due reviews and struggles on +the plan's own concepts, and milestones marked done with no evidence behind +them — and proposes *extend* while any count is above zero, *close* when all +three are zero, with one evidence line per counted item. The scheduler's +"new topic — start fresh" rows are not counted as due here: a topic you never +logged progress on is not a lapsed review, and a plan you have just finished +should not tell you to start fresh. Nothing about a plan's status changes +because of the review; `studyloop plan close PLAN_ID` launches the architect +with the same review as the first section of its brief, and the plan becomes +`complete` only when you agree in that conversation (the architect calls +`set_study_plan_status`). If the assessment cannot be read, the completion +action keeps its plain sentence and a warning says why — a failure is never +shown as a clean slate. This is plan-aware guidance with tested ranking rules — a bias, not a filter: an overdue review or a fresh struggle on an unrelated topic can still outrank new milestone work. With no active plan the recommendation is unchanged; a plan that cannot be read adds a warning and nothing else. The @@ -235,8 +248,10 @@ in the project's rubric receipt scored by the maintainer on 2026-09-16: the matching-due, urgent-unrelated and no-plan scenarios and the fully-checked plan's primary were accepted; the energy-deferred scenario (hands-on repair of a live struggle on a low-energy -day) and the completion action's wording were not, and are follow-on work -rather than edits to the ranking. +day) and the completion action's wording were not. The energy-deferred +scenario is still follow-on work rather than an edit to the ranking; the +completion action was reworked into the closing review described above, and +its re-run row awaits the maintainer's score. ## Deliberately not automatic @@ -266,7 +281,10 @@ refuses: the manual form's free-text brain dump is saved as context and never decomposed into the structured fields. The architect interview is the door for an agent-led decomposition, and the Web **Plan with architect** control carries its own optional brain dump to the architect as evidence — StudyLoop itself -still decomposes nothing. +still decomposes nothing. The same holds at the other end of a plan: checking +off the last milestone never completes it. The plan gets a closing review and +a proposal, `studyloop plan close` opens the conversation, and the status +moves to `complete` only when you agree in it. These boundaries are stated here so that a plan never appears more connected than it is. See the [roadmap](roadmap.md) for the intended continuity work. diff --git a/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md b/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md new file mode 100644 index 000000000..0cd41cf21 --- /dev/null +++ b/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md @@ -0,0 +1,74 @@ +## ADDED Requirements + +### Requirement: The completion action is a closing review, never a verdict +Rule 8's completion action for a fully-checked active plan (item 4 / D-G) +SHALL be composed from the plan's **end assessment**, read through the preview +path — `PlanApplication().assess(AssessPlan(plan_id, phase="end", +record=False))` — exactly once per fully-checked plan per `build_now_plan`. The +read SHALL write nothing: the document's bytes and status, the plans directory +and the checkpoint log are unchanged, and the recording writers +(`evaluate_and_record`, `record_checkpoint`) are never called. The engine +proposes; the architect asks; the learner decides; `set_study_plan_status` +remains the only door to `complete`. + +`CompletionAction` SHALL gain `due_reviews: int`, `struggles: int`, +`unverified_milestones: int`, `proposal: Literal["extend", "close"] | None` +and `evidence: tuple[str, ...]`, and SHALL keep `action`, the sentence every +renderer prints — now naming the proposal and the three counts and the one +door to acting on them, `studyloop plan close `; it SHALL differ from the +pre-change either-way sentence. The counts and lines SHALL come from one +definition, `planning.views.CompletionReview.from_evaluation`, consumed by both +this action and the `plan close` brief so the two surfaces never disagree: +`proposal == "extend"` iff any count is above zero, else `"close"`; one +evidence line per counted item, capped at `COMPLETION_EVIDENCE_CAP` (8) with a +final `… and N more` line. **Due reviews SHALL count only rows that name a +concept** (owner decision, 2026-09-17): the scheduler's `New topic -- start +fresh` row (`concept: None`, `evidence: configured_topic`) is a cold-start hint +for "what should I review now", not a lapsed review, and SHALL NOT be counted; +`plan evaluate` keeps the row, the exclusion is the completion review's. + +When the assessment fails, the recommendation SHALL NOT fail: the action +SHALL keep the plan-static sentence with `proposal` `None`, the counts `0` and +`evidence` empty, and `NowPlan.warnings` SHALL carry one entry naming the plan +and the failure, logged with its traceback first — so no renderer reads a +clean slate or outstanding work into a failure. The evaluation's own data-gap +warnings SHALL travel back into `warnings` prefixed with the plan id. + +The new keys SHALL appear only inside `completion_actions` entries, which +exist only when a fully-checked active plan exists; the no-plan payload stays +byte-identical to `tests/golden/now_plan_no_active.json`. Renderers SHALL +show the sentence (CLI `now`, the Today card, the daily recap), the CLI SHALL +print each evidence line beneath it, and none SHALL re-rank. + +#### Scenario: Due work on the plan's concepts proposes extend +- **WHEN** an active plan's every milestone is done and the end assessment + finds one due review on one of its concepts +- **THEN** `completion_actions[0]` carries `(due_reviews, struggles, + unverified_milestones) == (1, 0, 0)`, `proposal == "extend"`, an evidence + line naming the concept, and a sentence naming the plan and `extend`; the + JSON entry carries all five keys; no `study_plan:` candidate exists + +#### Scenario: A clean assessment proposes close +- **WHEN** the end assessment finds no due reviews, no struggles and every + done milestone backed by evidence +- **THEN** the counts are `(0, 0, 0)`, `proposal == "close"`, `evidence` is + empty and the sentence names `close` + +#### Scenario: New-topic rows are not due +- **WHEN** `spaced_repetition_due` returns only the `New topic -- start + fresh` row (`concept: None`) for the plan's topic and the concepts have + session mentions +- **THEN** `due_reviews == 0` and `proposal == "close"` + +#### Scenario: The ranker never changes a status +- **WHEN** `build_now_plan` runs against a fully-checked active plan with the + recording writers patched to raise +- **THEN** exactly one `AssessPlan(plan_id, "end", record=False)` intent is + assessed, the document's bytes are unchanged, the status is still `active` + and the checkpoint history is empty + +#### Scenario: A failed assessment keeps the sentence and warns +- **WHEN** `assess` raises for the fully-checked plan +- **THEN** `completion_actions[0].action` equals the pre-change sentence, + `proposal is None`, `warnings` names the plan and the failure, and the + primary is still the collected due item diff --git a/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md b/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md index d246effe2..8608bbd58 100644 --- a/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md +++ b/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md @@ -68,3 +68,49 @@ SHALL name both exits: `studyloop plan repair ` and `studyloop plan status launch; the second exits `1` naming `nope` with no traceback; the third exits `1` and names both `studyloop plan status husk paused` and `studyloop plan repair husk` + +### Requirement: The plan CLI closes a fully-checked plan through the one launch chain, consensually +`studyloop plan close ` (item 4 / D-G) SHALL be the architect launch and +never a second path — the sibling of `plan repair`, through the same +`ctx.invoke(study, …, mode="plan-architect", topic=, +brief=…, brief_intro=…)`. It SHALL `_inspect(id)` (unknown id → the seam's +not-found through `_fail_for`, exit `1`); SHALL exit `1` with `'' still +has N open milestone(s)` and no launch while any milestone is open; SHALL +exit `1` with a pointer to `studyloop plan architect` for a plan with no +milestones; SHALL exit `0` with no launch for a plan that is already +`complete`; and for a fully-checked plan SHALL run the end assessment as a +**preview** (`AssessPlan(phase="end", record=False)`) and launch once. The +command itself SHALL write nothing: the document, the plans directory, the +plan's status and the checkpoint log are unchanged after it returns; the +status moves to `complete` only when the learner agrees in the launched +session and the architect calls `set_study_plan_status`. + +The brief's first section SHALL be `### Closing review`, whose first four +`- ` lines are `Due reviews on plan concepts: N`, `Struggles on plan +concepts: N`, `Unverified milestones: N` and `Proposal: extend|close`, +followed by one `- ` evidence line per counted item — the same +`CompletionReview` the `now` engine puts on its completion action, so the two +never disagree on a count (new-topic rows excluded) — then the plan as it +stands (title, id, status, topics, milestones done/total, created), and a +`### Data gaps` section only when the evaluation reported a reader +unavailable. The `brief_intro` SHALL say `CLOSING REVIEW` and `only when the +learner agrees`, and SHALL NOT say `build a study plan`. + +#### Scenario: plan close on a fully-checked plan launches once with the review first and writes nothing +- **WHEN** `plan close glue-etl` is run on an active plan whose two milestones + are both done, with the due reader returning one real due row on a plan + concept and one `New topic -- start fresh` row (`concept: None`) +- **THEN** exactly one `start_session` call is made with `mode="plan-architect"` + and `topic` equal to the plan's title; the `### Closing review` section's + first four lines are `Due reviews on plan concepts: 1`, `Struggles on plan + concepts: 0`, `Unverified milestones: 0`, `Proposal: extend`, followed by an + evidence line naming the due concept; the brief says `2/2`; the intro says + `CLOSING REVIEW` and `only when the learner agrees` and not `build a study + plan`; the plans directory, the plan's `active` status and the checkpoint + history are unchanged + +#### Scenario: plan close on an unfinished plan refuses without launching +- **WHEN** `plan close glue-etl` is run on an active plan with two open + milestones +- **THEN** it exits `1` with `'glue-etl' still has 2 open milestone(s)`, no + traceback, no launch, and the plans directory unchanged diff --git a/packages/studyloop/src/studyloop/cli/_now.py b/packages/studyloop/src/studyloop/cli/_now.py index ce6a3bdef..938667ab1 100644 --- a/packages/studyloop/src/studyloop/cli/_now.py +++ b/packages/studyloop/src/studyloop/cli/_now.py @@ -71,6 +71,10 @@ def _render_plan(plan) -> None: ) for completion in getattr(plan, "completion_actions", ()): console.print(f"[green]Plan complete:[/green] {escape(completion.action)}") + # The review's evidence, one dim line per counted item (D-G); the + # sentence above already carries the proposal and the counts. + for line in getattr(completion, "evidence", ()): + console.print(f" [dim]• {escape(line)}[/dim]") for warning in getattr(plan, "warnings", ()): console.print(f"[dim]Plan warning: {escape(warning)}[/dim]") diff --git a/packages/studyloop/src/studyloop/cli/_plan.py b/packages/studyloop/src/studyloop/cli/_plan.py index 623360ec9..46a0f7d27 100644 --- a/packages/studyloop/src/studyloop/cli/_plan.py +++ b/packages/studyloop/src/studyloop/cli/_plan.py @@ -30,6 +30,7 @@ from studyloop.planning import ( PLAN_STATUSES, AssessPlan, + CompletionReview, CreatePlan, InvalidField, InvalidMilestone, @@ -48,7 +49,7 @@ ) if TYPE_CHECKING: - from studyloop.planning import AssessmentResult, PlanDetail, PlanDetailIntent + from studyloop.planning import AssessmentResult, PlanDetail, PlanDetailIntent, PlanSummary def _fail(message: str) -> NoReturn: @@ -543,6 +544,20 @@ def plan_architect(ctx: click.Context, agent: str | None) -> None: ) +def _render_plan_as_it_stands(s: PlanSummary) -> str: + """The ``### The plan as it stands`` section both launch briefs carry.""" + topics = ", ".join(s.topics) if s.topics else "(none)" + return ( + "### The plan as it stands\n\n" + f"- Title: {s.title}\n" + f"- Id: {s.plan_id}\n" + f"- Status: {s.status}\n" + f"- Topics: {topics}\n" + f"- Milestones: {s.milestone_done}/{s.milestone_total} done\n" + f"- Created: {s.created}\n" + ) + + def _render_repair_brief(detail: PlanDetail) -> str: """The brief ``plan repair`` hands the architect: blockers first, then the plan as it stands. @@ -555,17 +570,10 @@ def _render_repair_brief(detail: PlanDetail) -> str: s = detail.summary blockers = "\n".join(f"- {item}" for item in detail.readiness.blockers) - topics = ", ".join(s.topics) if s.topics else "(none)" return ( "### Repair: what this plan is missing\n\n" f"{blockers}\n\n" - "### The plan as it stands\n\n" - f"- Title: {s.title}\n" - f"- Id: {s.plan_id}\n" - f"- Status: {s.status}\n" - f"- Topics: {topics}\n" - f"- Milestones: {s.milestone_done}/{s.milestone_total} done\n" - f"- Created: {s.created}\n\n" + f"{_render_plan_as_it_stands(s)}\n" f"{husk_provenance(s.created)}\n" ) @@ -629,6 +637,113 @@ def plan_repair(ctx: click.Context, plan_id: str, agent: str | None) -> None: ) +#: The sentence that frames a closing-review brief in place of the planning one. +CLOSE_BRIEF_INTRO = ( + "This is a CLOSING REVIEW session: every milestone of the plan below is checked off — " + "read the evidence back to the learner, propose extending or closing, ask what they are " + "not comfortable with, and change the plan's status only when the learner agrees." +) + + +def _render_closing_brief( + detail: PlanDetail, review: CompletionReview, gaps: tuple[str, ...] +) -> str: + """The brief ``plan close`` hands the architect: the closing review first, then the plan. + + The first section's first four ``- `` lines are the three counts and the + proposal, followed by one evidence line per counted item — readable off + the top without parsing prose, as the repair brief's blockers are. The + review is the same :class:`~studyloop.planning.CompletionReview` the + ``now`` engine puts on its completion action: one definition, two surfaces. + A ``### Data gaps`` section appears only when the evaluation reported a + reader unavailable, so the agent knows the counts are partial. + """ + lines = [ + f"Due reviews on plan concepts: {review.due_reviews}", + f"Struggles on plan concepts: {review.struggles}", + f"Unverified milestones: {review.unverified_milestones}", + f"Proposal: {review.proposal}", + *review.evidence, + ] + brief = ( + "### Closing review\n\n" + + "\n".join(f"- {line}" for line in lines) + + "\n\n" + + _render_plan_as_it_stands(detail.summary) + ) + if gaps: + brief += "\n### Data gaps\n\n" + "\n".join(f"- {gap}" for gap in gaps) + "\n" + return brief + + +@plan_group.command("close") +@click.argument("plan_id") +@click.option( + "--agent", + "-a", + help="AI agent to launch (auto-detects if omitted).", +) +@click.pass_context +def plan_close(ctx: click.Context, plan_id: str, agent: str | None) -> None: + """Review a fully-checked plan with the architect and decide: extend it or close it. + + A plan whose every milestone is checked is finished work, not yet a + finished plan. This runs the end assessment as a preview — due reviews, + struggles and milestones marked done without evidence, counted on the + plan's own concepts — and launches the study-plan-architect (the same + ``studyloop study --mode plan-architect`` chain as ``plan architect`` and + ``plan repair``, never a second path) with those counts, the proposal they + imply and the evidence as the first section of its brief. The command + itself writes nothing: no checkpoint is recorded, and the status changes + only when the learner agrees in that session (``set_study_plan_status``). + + A plan with open milestones has nothing to close yet (exit 1, naming how + many are open); a plan that is already ``complete`` is left alone. + """ + detail = _inspect(plan_id) + s = detail.summary + if s.status == "complete": + console.print(f"[dim]{s.plan_id!r} is already complete.[/dim]") + return + if s.milestone_total == 0: + _fail( + f"{s.plan_id!r} has no milestones, so there is nothing to close — finish it with " + "studyloop plan architect." + ) + open_count = s.milestone_total - s.milestone_done + if open_count: + _fail( + f"{s.plan_id!r} still has {open_count} open milestone(s) — nothing to close yet. " + f"Tick each as the learner demonstrates it: " + f"studyloop plan milestone {s.plan_id} INDEX --done" + ) + + result = _assess(AssessPlan(plan_id=s.plan_id, phase="end", record=False)) + review = CompletionReview.from_evaluation(result.evaluation) + + from studyloop.cli._study import study + + console.print( + f"[green]{s.plan_id!r} ({s.title}) has every milestone checked; the closing review " + f"proposes: {review.proposal}. Launching the architect to decide with you.[/green]" + ) + ctx.invoke( + study, + topic=s.title, + agent=agent, + mode="plan-architect", + timer=None, + energy=5, + web=False, + lan=False, + password="", + resume=False, + end_session=False, + brief=_render_closing_brief(detail, review, result.warnings), + brief_intro=CLOSE_BRIEF_INTRO, + ) + + @plan_group.command("path") def plan_path_cmd() -> None: """Print the directory holding plan documents.""" diff --git a/packages/studyloop/src/studyloop/learning/decision.py b/packages/studyloop/src/studyloop/learning/decision.py index ff73aa91f..7d80aeccf 100644 --- a/packages/studyloop/src/studyloop/learning/decision.py +++ b/packages/studyloop/src/studyloop/learning/decision.py @@ -4,7 +4,11 @@ it as one plan-static read — ``PlanApplication().get_active_guidance()`` — and leave as a *bias* on the existing scores, a synthesised candidate for an unrepresented next milestone, and references attached to the ranked actions. -Renderers show that plan relevance; none of them re-rank. +The one plan that is not plan-static is a fully-checked one (rule 9): its +completion action carries the end assessment's completion review, read through +the preview path (``assess(AssessPlan(phase="end", record=False))``) — one +call per such plan, no write, no status change (D-G). Renderers show that plan +relevance; none of them re-rank. With no active plan the emitted JSON is byte for byte what it was before plans existed: every additive field is omitted when empty @@ -25,7 +29,13 @@ if TYPE_CHECKING: from datetime import date - from studyloop.planning.views import ActiveGuidance, ActivePlanGuidance, MilestoneView + from studyloop.planning.views import ( + ActiveGuidance, + ActivePlanGuidance, + CompletionReview, + MilestoneView, + PlanSummary, + ) logger = logging.getLogger(__name__) @@ -121,14 +131,33 @@ def to_json_dict(self) -> dict: @dataclass(frozen=True) class CompletionAction: - """What to do about an active plan whose every milestone is checked (rule 9).""" + """What to do about an active plan whose every milestone is checked (rule 9). + + ``action`` is the sentence every renderer prints. Since D-G (item 4) it is + composed from the end assessment's completion review — the three counts + on the plan's own concepts and the proposal they imply — read through the + preview path, ``assess(AssessPlan(phase="end", record=False))``: no write, + no checkpoint, no status change. ``proposal`` is ``None`` when that + assessment failed: the counts are then *unknown*, not zero — ``action`` + falls back to the plan-static sentence and ``NowPlan.warnings`` says why — + so no renderer reads a clean slate or outstanding work into a failure. The + engine proposes; the architect asks; the learner decides; + ``set_study_plan_status`` is the only door to ``complete``. + """ plan_id: str plan_title: str action: str + due_reviews: int = 0 + struggles: int = 0 + unverified_milestones: int = 0 + proposal: Literal["extend", "close"] | None = None + evidence: tuple[str, ...] = () def to_json_dict(self) -> dict: - return asdict(self) + data = asdict(self) + data["evidence"] = list(self.evidence) + return data @dataclass(frozen=True) @@ -698,6 +727,82 @@ def _load_guidance(today: date) -> ActiveGuidance | None: return None +def _review_completion(plan_id: str) -> tuple[CompletionReview | None, tuple[str, ...]]: + """The end assessment's completion review for one fully-checked plan (rule 9, D-G). + + The preview path — ``AssessPlan(phase="end", record=False)`` — so the + document, its status and the checkpoint log are untouched; exactly one + call per fully-checked plan per ``build_now_plan``. A failure degrades to + ``None`` plus one learner-facing warning naming the plan (the + recommendation never fails on a plan), logged with its traceback first so + a programming error cannot hide behind it, as :func:`_load_guidance` does. + The evaluation's own data-gap warnings travel back prefixed with the plan + id: a count read while one of its readers was unavailable is partial, and + the learner should know that rather than read it as zero. + """ + try: + from studyloop.planning import AssessPlan, CompletionReview + from studyloop.planning.application import PlanApplication + + result = PlanApplication().assess(AssessPlan(plan_id=plan_id, phase="end", record=False)) + except Exception as exc: + logger.warning( + "active plan %r could not be assessed for completion", plan_id, exc_info=True + ) + return None, ( + f"active plan {plan_id!r} could not be assessed for completion ({exc}); " + "shown without its counts", + ) + gaps = tuple(f"active plan {plan_id!r}: {warning}" for warning in result.warnings) + return CompletionReview.from_evaluation(result.evaluation), gaps + + +def _completion_sentence(plan_id: str, title: str, review: CompletionReview) -> str: + """The completion action's sentence, composed from the review's proposal (D-G). + + Names the proposal and the three counts, then the one door to acting on + it — ``studyloop plan close ``, where the architect walks the evidence + with the learner. Spoken by the recap as well as printed, so no markup. + """ + + def plural(count: int, noun: str) -> str: + return f"{count} {noun}{'' if count == 1 else 's'}" + + if review.proposal == "close": + return ( + f"Every milestone of {title!r} is checked off and the closing review is clean — " + "it proposes closing the plan. Close it with the architect when you agree: " + f"studyloop plan close {plan_id}." + ) + counts = ( + f"{plural(review.due_reviews, 'due review')}, {plural(review.struggles, 'struggle')} and " + f"{plural(review.unverified_milestones, 'unverified milestone')} on its concepts" + ) + return ( + f"Every milestone of {title!r} is checked off, and the closing review proposes " + f"extending the plan — {counts}. Walk the evidence with the architect: " + f"studyloop plan close {plan_id}." + ) + + +def _completion_action( + summary: PlanSummary, fallback: str, review: CompletionReview | None +) -> CompletionAction: + """Rule 9's entry: the reviewed action, or the plan-static sentence when unassessed.""" + if review is None: + return CompletionAction(plan_id=summary.plan_id, plan_title=summary.title, action=fallback) + return CompletionAction( + plan_id=summary.plan_id, + plan_title=summary.title, + action=_completion_sentence(summary.plan_id, summary.title, review), + due_reviews=review.due_reviews, + struggles=review.struggles, + unverified_milestones=review.unverified_milestones, + proposal=review.proposal, + evidence=review.evidence, + ) + + def _milestone_concept_keys(plan: ActivePlanGuidance) -> frozenset[str]: if plan.next_milestone is None: return frozenset() @@ -722,7 +827,10 @@ class _PlanContext: ``matchable`` are the plans that may bias and be referenced by a candidate: every active plan except a fully-checked one, whose work is - done and which is represented by a completion action instead (rule 9). + done and which is represented by a completion action instead (rule 9) — + the one entry built from a second seam read, the end assessment's preview + (:func:`_review_completion`), so it can propose ``extend`` or ``close`` + from evidence rather than either way (D-G). ``synthesise`` are the plans whose next milestone may become a candidate when nothing collected represents it (rule 6): ready, with a next milestone, and within the energy capability (rule 3). @@ -778,13 +886,9 @@ def build(cls, guidance: ActiveGuidance | None, *, energy: EnergyLevel) -> _Plan eligible = False if plan.completion_action: - completions.append( - CompletionAction( - plan_id=summary.plan_id, - plan_title=summary.title, - action=plan.completion_action, - ) - ) + review, notes = _review_completion(summary.plan_id) + warnings.extend(notes) + completions.append(_completion_action(summary, plan.completion_action, review)) else: matchable.append(plan) keys.update(plan.match_keys) diff --git a/packages/studyloop/src/studyloop/planning/__init__.py b/packages/studyloop/src/studyloop/planning/__init__.py index e5bcac2a5..95c2e6ae5 100644 --- a/packages/studyloop/src/studyloop/planning/__init__.py +++ b/packages/studyloop/src/studyloop/planning/__init__.py @@ -96,6 +96,8 @@ AssessmentResult, CheckpointHistoryView, CheckpointView, + CompletionProposal, + CompletionReview, DeleteResult, InterviewItemView, LearningRecordOutcome, @@ -126,6 +128,8 @@ "Checkpoint", "CheckpointHistoryView", "CheckpointView", + "CompletionProposal", + "CompletionReview", "ConceptEvidence", "CreatePlan", "DeletePlan", diff --git a/packages/studyloop/src/studyloop/planning/views.py b/packages/studyloop/src/studyloop/planning/views.py index 9c67e835e..aea6e791a 100644 --- a/packages/studyloop/src/studyloop/planning/views.py +++ b/packages/studyloop/src/studyloop/planning/views.py @@ -744,6 +744,88 @@ def to_json_dict(self) -> dict[str, Any]: } +#: What the completion review proposes: ``extend`` while any of its counts is +#: above zero, ``close`` when all three are zero. A proposal, never a verdict. +CompletionProposal = Literal["extend", "close"] + +#: Upper bound on the evidence lines a completion review carries — one per +#: counted item, then a single line saying how many more the counts cover. +#: Enough to read off the top of a brief or a card; the counts stay exact. +COMPLETION_EVIDENCE_CAP = 8 + + +@dataclass(frozen=True) +class CompletionReview: + """The completion review's reading of an ``end`` assessment (D-G, item 4). + + One definition for the two surfaces that say what to do with an active plan + whose every milestone is checked — the ``now`` engine's completion action + and the ``plan close`` brief — so they never disagree on a count. Three + counts on the plan's own concepts, the proposal they imply, and one + evidence line per counted item (capped at :data:`COMPLETION_EVIDENCE_CAP`, + then one line saying how many more). Nothing here changes a status, and + nothing may because of it: the engine proposes, the architect asks, the + learner decides. + + **Due reviews count only rows that name a concept** (owner decision, + 2026-09-17). :func:`~studyloop.history.spaced_repetition_due` appends a + ``New topic -- start fresh`` row (``concept: None``) for every plan topic + with no progress rows — the scheduler's cold-start hint for "what should I + review now", not a lapsed review. The evaluator keeps it (``plan evaluate + --phase start`` wants it) and already ignores it at concept level, where a + ``None`` concept never matches a milestone concept; counting it here would + tell a learner who has just ticked every milestone to "start fresh". + ``unverified_milestones`` remains the honest carrier of "done without + evidence". + + The counts are bounded by the evaluation's own row caps + (:func:`~studyloop.planning.evaluation.evaluate_plan` keeps ten due rows and + ten struggle rows): a plan with more outstanding work than that reads as + ten — still ``extend``. + """ + + due_reviews: int + struggles: int + unverified_milestones: int + proposal: CompletionProposal + evidence: tuple[str, ...] + + @classmethod + def from_evaluation(cls, evaluation: PlanEvaluationView) -> CompletionReview: + due = [row for row in evaluation.due_reviews if row.get("concept")] + lines: list[str] = [] + for row in due: + kind = str(row.get("review_type") or "").strip() + label = f"Due review: {row['concept']}" + lines.append(f"{label} — {kind}" if kind else label) + for row in evaluation.struggles: + lines.append(f"Struggle: {row.get('concept') or row.get('topic')}") + for title in evaluation.unverified_milestones: + lines.append( + f"Unverified milestone: {title} — marked done, no evidence on its concepts" + ) + if len(lines) > COMPLETION_EVIDENCE_CAP: + more = len(lines) - COMPLETION_EVIDENCE_CAP + lines = [*lines[:COMPLETION_EVIDENCE_CAP], f"… and {more} more"] + counts = (len(due), len(evaluation.struggles), len(evaluation.unverified_milestones)) + return cls( + due_reviews=counts[0], + struggles=counts[1], + unverified_milestones=counts[2], + proposal="extend" if any(counts) else "close", + evidence=tuple(lines), + ) + + def to_json_dict(self) -> dict[str, Any]: + return { + "due_reviews": self.due_reviews, + "struggles": self.struggles, + "unverified_milestones": self.unverified_milestones, + "proposal": self.proposal, + "evidence": list(self.evidence), + } + + @dataclass(frozen=True) class ActivePlanGuidance: """What the ``now`` ranker needs to know about one active plan (D-5). diff --git a/packages/studyloop/src/studyloop/web/static/index.html b/packages/studyloop/src/studyloop/web/static/index.html index bea1ff5d5..2a832a8a6 100644 --- a/packages/studyloop/src/studyloop/web/static/index.html +++ b/packages/studyloop/src/studyloop/web/static/index.html @@ -1113,6 +1113,9 @@

+ diff --git a/packages/studyloop/src/studyloop/web/static/js/components/today-panel.js b/packages/studyloop/src/studyloop/web/static/js/components/today-panel.js index 12d0ee372..dcef81af7 100644 --- a/packages/studyloop/src/studyloop/web/static/js/components/today-panel.js +++ b/packages/studyloop/src/studyloop/web/static/js/components/today-panel.js @@ -167,6 +167,16 @@ export function todayPanel() { return actions.map((a) => a.action); }, + /* The closing review's evidence (D-G, item 4): one line per counted item + across every completion action, in the engine's order — what the + proposal in the sentence rests on. A pre-D-G entry without `evidence` + contributes nothing, and a failed assessment (`proposal` null) carries + none by construction. */ + completionEvidence() { + const actions = (this.plan && this.plan.completion_actions) || []; + return actions.flatMap((a) => (Array.isArray(a.evidence) ? a.evidence : []).map(String)); + }, + /* The engine's warnings, verbatim: an active plan that is not ready (its blockers, "pause or repair"), a document that could not be read. Data the CLI and the JSON already show; the card shows it too. */ diff --git a/packages/studyloop/src/studyloop/web/static/style.css b/packages/studyloop/src/studyloop/web/static/style.css index 1e4b8ab22..3d4d11432 100644 --- a/packages/studyloop/src/studyloop/web/static/style.css +++ b/packages/studyloop/src/studyloop/web/static/style.css @@ -3814,6 +3814,8 @@ body[data-palette="everforest"] { } .today-concept { margin: 0 0 6px; font-size: 1.4rem; } .today-meta { margin: 0 0 10px; color: var(--text-muted); } +/* The closing review's evidence lines under a "Plan complete" note (D-G). */ +.today-plan-evidence { margin: -6px 0 6px 16px; font-size: 0.9rem; } .today-reason { margin: 0 0 18px; } .today-start-btn { font-size: 1.05rem; padding: 10px 22px; } .today-resume { margin-bottom: 16px; } diff --git a/packages/studyloop/tests/js/today-panel-plan.test.js b/packages/studyloop/tests/js/today-panel-plan.test.js index 2bef98833..71d840a76 100644 --- a/packages/studyloop/tests/js/today-panel-plan.test.js +++ b/packages/studyloop/tests/js/today-panel-plan.test.js @@ -131,12 +131,45 @@ test('completionNotes: the engine\u2019s completion actions, verbatim', () => { assert.equal(panel.hasPlanContext, true); }); +test('completionEvidence: the closing review\u2019s lines, in the engine\u2019s order, across actions', () => { + const panel = todayPanel(); + panel.plan = { + ...NO_PLAN_PAYLOAD, + completion_actions: [ + { + plan_id: 'done', + plan_title: 'Done', + action: 'closing review proposes extending the plan', + due_reviews: 1, + struggles: 0, + unverified_milestones: 1, + proposal: 'extend', + evidence: [ + 'Due review: alpha \u2014 overdue', + 'Unverified milestone: B \u2014 marked done, no evidence on its concepts', + ], + }, + // A pre-D-G entry (no evidence key) and a failed assessment (proposal null, + // evidence empty) both contribute nothing. + { plan_id: 'old', plan_title: 'Old', action: 'plain sentence' }, + { plan_id: 'unread', plan_title: 'Unread', action: 'plain sentence', proposal: null, evidence: [] }, + ], + }; + + assert.deepEqual(panel.completionEvidence(), [ + 'Due review: alpha \u2014 overdue', + 'Unverified milestone: B \u2014 marked done, no evidence on its concepts', + ]); + assert.equal(panel.completionNotes().length, 3); +}); + test('a payload without plan keys renders no plan text, before and after init-like assignment', () => { const panel = todayPanel(); assert.equal(panel.planLabel(null), ''); assert.deepEqual(panel.deferredNotes(), []); assert.deepEqual(panel.completionNotes(), []); + assert.deepEqual(panel.completionEvidence(), []); assert.equal(panel.hasPlanContext, false); panel.plan = NO_PLAN_PAYLOAD; diff --git a/packages/studyloop/tests/test_now_plan_guidance.py b/packages/studyloop/tests/test_now_plan_guidance.py index b7a3da01c..5fa9a3d63 100644 --- a/packages/studyloop/tests/test_now_plan_guidance.py +++ b/packages/studyloop/tests/test_now_plan_guidance.py @@ -919,9 +919,9 @@ def test_completion_action_carries_the_end_assessment_and_proposes_extend_when_c [action] = plan.completion_actions assert action.plan_id == "done-plan" - assert (action.due_reviews, action.struggles, action.unverified_milestones) == (1, 0, 0) # pyright: ignore[reportAttributeAccessIssue] - assert action.proposal == "extend" # pyright: ignore[reportAttributeAccessIssue] - assert any("alpha" in line for line in action.evidence), action.evidence # pyright: ignore[reportAttributeAccessIssue] + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (1, 0, 0) + assert action.proposal == "extend" + assert any("alpha" in line for line in action.evidence), action.evidence assert "Done Plan" in action.action assert "extend" in action.action.lower() assert action.action != _pre_change_sentence("Done Plan") @@ -946,9 +946,9 @@ def test_completion_action_proposes_close_when_the_assessment_is_clean(monkeypat plan = build_now_plan() [action] = plan.completion_actions - assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) # pyright: ignore[reportAttributeAccessIssue] - assert action.proposal == "close" # pyright: ignore[reportAttributeAccessIssue] - assert action.evidence == () # pyright: ignore[reportAttributeAccessIssue] + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) + assert action.proposal == "close" + assert action.evidence == () assert "Done Plan" in action.action assert "close" in action.action.lower() assert action.action != _pre_change_sentence("Done Plan") @@ -984,9 +984,9 @@ def test_completion_review_does_not_count_new_topic_rows_as_due(monkeypatch) -> plan = build_now_plan() [action] = plan.completion_actions - assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) # pyright: ignore[reportAttributeAccessIssue] - assert action.proposal == "close" # pyright: ignore[reportAttributeAccessIssue] - assert action.evidence == () # pyright: ignore[reportAttributeAccessIssue] + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) + assert action.proposal == "close" + assert action.evidence == () def test_completion_never_changes_status(monkeypatch) -> None: From 7d6980673500494093ae4da90e4a17384b694130 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 09:16:27 +0100 Subject: [PATCH 04/23] docs(plan-integration): tick T4.2/T4.3 with their receipts; record item 4's GREEN-time decisions T4.2 GREEN is 82293293 (7 RED -> green, matched-control full suite with zero regressions, 44 environmental ids now committed by name); T4.3's rubric row 4b is in the same commit with the verdict pending the owner. Design section 4 closes its one open decision - `proposal` is nullable, None on a failed assessment, represented once at the action - and records the two decisions taken in GREEN: the Today card renders the review's evidence lines, and the docs' pinned six-boundary list stays at six with the consensual close stated beside it. --- .../plan-integration-followons/design.md | 22 ++++++++++++++----- .../plan-integration-followons/tasks.md | 20 +++++++++++++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/openspec/changes/plan-integration-followons/design.md b/openspec/changes/plan-integration-followons/design.md index 98b1fc6f0..08793e22c 100644 --- a/openspec/changes/plan-integration-followons/design.md +++ b/openspec/changes/plan-integration-followons/design.md @@ -175,11 +175,23 @@ among what MCP revises and the row names every schema property. completion review's, one definition beside `PlanEvaluationView` in `planning/views.py`, consumed by both the engine's `CompletionAction` and the `plan close` brief. Measured cost of the preview on the live 877 MB database: ~320 ms per fully-checked plan per `build_now_plan` (five readers), a transient state by design. -- **Open for GREEN (not pinned):** what `proposal` holds when the assessment fails. `Literal["extend", "close"]` - has no honest value for "not assessed" — `extend` asserts outstanding work without evidence, `close` asserts - a clean slate without evidence. Default unless vetoed: `proposal: Literal["extend", "close"] | None`, `None` - on failure with the counts `0` and `evidence` empty; the `warnings` entry explains; renderers print the plain - sentence when `proposal is None`. One nullable field carries the state; the three counts keep their type. +- **Decided in GREEN (`82293293`, 2026-09-18; unvetoed):** `proposal` when the assessment fails. + `Literal["extend", "close"]` has no honest value for "not assessed" — `extend` asserts outstanding work + without evidence, `close` asserts a clean slate without evidence. Shipped as `proposal: Literal["extend", + "close"] | None`, `None` on failure with the counts `0` and `evidence` empty; the `warnings` entry explains; + renderers print the plain sentence when `proposal is None`. One nullable field carries the state; the three + counts keep their type. The `CompletionReview` value object itself stays non-nullable (`proposal: + CompletionProposal`): a review exists only when an evaluation did, and the engine's `_review_completion` + returns `None` for the whole review on failure — so "unassessed" is represented once, at the action, not + twice. +- **Two more GREEN-time decisions (`82293293`):** (1) the Today card gained `completionEvidence()` and renders + the review's evidence lines under the "Plan complete" note, matching CLI `now`'s dim lines — the design said + the card prints "the proposal and the counts", which the sentence carries, but the surface most learners read + should also show what the proposal rests on; a pre-D-G entry without `evidence`, or a failed assessment, + contributes nothing. (2) `docs/study-plans.md`'s "Deliberately not automatic" list is the pinned six-item + `NOT_AUTOMATIC` constant from issue #7's out-of-scope list (`test_not_automatic_constant_is_well_formed` + asserts exactly six); the consensual close is therefore stated in the prose beside the list, as the + brain-dump limit is, rather than as a seventh boundary. ## 5. Item 5 — per-item energy demand and the body-doubling floor (D-F) — designed here, reviewed separately diff --git a/openspec/changes/plan-integration-followons/tasks.md b/openspec/changes/plan-integration-followons/tasks.md index dbc0412fd..53a2267a5 100644 --- a/openspec/changes/plan-integration-followons/tasks.md +++ b/openspec/changes/plan-integration-followons/tasks.md @@ -129,10 +129,26 @@ writer, through the existing gate, closes that. Kept out of item 3 so item 3's f `test_completion_assessment_failure_keeps_the_sentence_and_warns`; `tests/test_cli_plan_seam.py::test_plan_close_launches_the_architect_with_the_assessment_in_the_brief`, `::test_plan_close_on_an_unfinished_plan_refuses`; golden byte-identity test still green. DoD: RED committed. -- [ ] **T4.2** GREEN: `CompletionAction` fields + `_PlanContext.build` preview assessment; renderers (CLI `now`, +- [x] **T4.2** (GREEN `82293293`: 7 RED → green; two-file run 64 passed; full suite 30 failed / 7233 passed / + 4 skipped / 14 errors in 952 s, `comm` against a clean `f1c52ce8` control run in parallel: item4 − control + = ∅, control − item4 = exactly the 7 REDs, 44 shared environmental ids committed by name in + `receipts/full-suite-control-item4-2026-09-18.md` (items 3/3b's 45-id list was never persisted, so the + one that differs cannot be named); golden sha `ec451ce8…` unchanged; `just test-js` 136/136 (+1: Today + card `completionEvidence()`); `just lint` clean, `just typecheck` 0 errors, `mkdocs build --strict` clean, + `openspec validate` valid; hooks first time. One decision taken in GREEN, recorded in design §4: `proposal` + is `Literal["extend", "close"] | None`, `None` on a failed assessment. The Today card gained the evidence + lines beside the CLI's, so the surface most learners read shows what the proposal rests on. The docs' + "Deliberately not automatic" list stayed at the pinned six `NOT_AUTOMATIC` boundaries; the consensual + close is stated in the prose beside it, as the brain-dump limit is.) + GREEN: `CompletionAction` fields + `_PlanContext.build` preview assessment; renderers (CLI `now`, recap, Today `completionNotes`); `plan close`; persona "Extend or close" subsection; docs; spec deltas. DoD: full suite; golden sha unchanged; `just test-js`. -- [ ] **T4.3** Rubric: add row **4b** to `receipts/now-rubric-2026-09-16.md` (do not overwrite row 4) with the +- [x] **T4.3** (in `82293293`: row **4b** added under row 4 — row 4 untouched — with the re-run primary + (`decorators` 118, unchanged) and both readings of the completion action as the engine emitted them + through the RED tests' own fixtures: (a) one due review on plan concept `alpha` → `extend`, evidence + `Due review: alpha — overdue`; (b) clean → `close`, evidence empty. Verdict `PENDING` for the owner; the + receipt's status line and re-run mapping name the two backing tests.) Rubric: add row **4b** to + `receipts/now-rubric-2026-09-16.md` (do not overwrite row 4) with the re-run primary and the proposal; verdict column `PENDING` for the owner. ## ⚖ Council review 6 — items 1–4 From 9d10fee663083566a227420594bde99bd3f8264b Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 10:21:10 +0100 Subject: [PATCH 05/23] =?UTF-8?q?docs(plan-integration):=20record=20the=20?= =?UTF-8?q?owner's=20row=204b=20verdicts=20=E2=80=94=20yes=20on=20both=20r?= =?UTF-8?q?eadings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The owner scored rubric row 4b on 2026-09-18: (a) the evidence-backed 'extend' proposal is one they would walk, (b) a clean review's 'close' is one they would agree to. This closes row 4's 'no as phrased' finding from 2026-09-16; row 4 stays as the record of the original finding. Recorded in the receipt's status line and verdict cell exactly as given (no rationale was supplied beyond the two answers, and none is invented), in tasks.md (T4.3 note; T5.5's archive condition now names 3b as the one row still outstanding), and on the public study-plans page, which said the re-run row 'awaits the maintainer's score'. The contract pin keys on 'owner verdicts RECORDED' and stays satisfied. --- .../plan-integration/receipts/now-rubric-2026-09-16.md | 4 ++-- docs/study-plans.md | 3 ++- openspec/changes/plan-integration-followons/tasks.md | 6 ++++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md b/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md index a657c5d5e..ff46f1f47 100644 --- a/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md +++ b/docs/architecture/plan-integration/receipts/now-rubric-2026-09-16.md @@ -1,6 +1,6 @@ # Plan-aware `now` — five-scenario human rubric (D-16) · 2026-09-16 -**Status: owner verdicts RECORDED 2026-09-16 (interactive walkthrough with the coordinator).** Scenarios 1, 2 and the primary of 4: yes. Scenario 3: **no** (finding). Scenario 4 completion action: **no as phrased** (finding). Scenario 5: verified. **Row 4b added 2026-09-18** (item 4 / D-G, tree `feat/plan-close`): scenario 4 re-run with the end assessment planted, both proposals recorded as emitted; verdict `PENDING` for the owner — row 4 is kept as the record of the original finding. This receipt was produced unattended +**Status: owner verdicts RECORDED 2026-09-16 (interactive walkthrough with the coordinator).** Scenarios 1, 2 and the primary of 4: yes. Scenario 3: **no** (finding). Scenario 4 completion action: **no as phrased** (finding). Scenario 5: verified. **Row 4b added and scored 2026-09-18** (item 4 / D-G, tree `feat/plan-close`; outputs emitted from the tree committed as `82293293`): scenario 4 re-run with the end assessment planted, both proposals recorded as emitted; owner verdict **yes** on both readings — (a) *extend* with named evidence is a proposal the owner would walk, (b) a clean review's *close* is one the owner would agree to. This closes row 4's "no as phrased" finding; row 4 is kept as the record of the original finding. This receipt was produced unattended overnight. Every scenario below was *run* on frozen fixtures and the primary and its rationale are recorded exactly as the engine emitted them; the "would I do the primary?" column is a human judgement that only the owner can @@ -31,7 +31,7 @@ learning"). | 2 | Urgent-unrelated wins | Same plan. One due item: `decorators`/python, base 100 (an overdue spaced-repetition review). Nothing represents milestone 0. | **`decorators`** (118, no refs); alternate `window function` (60, `source=study_plan:sql-windows:0`, `plan_refs=[(sql-windows, 0)]`, reason "Next milestone 1/1 of plan 'SQL Windows': Window basics"). | Rule 5: the unrelated candidate is in a more urgent class (due review) and wins outright — the bias cannot lift new-milestone work over it. Rule 6: the plan's next milestone was unrepresented, so it was synthesised at base 48 + bias 12 = 60 and appears as the plan-backed alternate (rule 8 satisfied without any swap). | **yes** — owner, 2026-09-16: clear the overdue review first. Note for follow-on: an overdue item *unrelated* to the plan must not sit as an alternate indefinitely — propose it explicitly (age-aware nudge) or let the learner retire it. | | 3 | Energy-deferred | Plan `sql-windows` with `energy_floor: 5`; milestone 0 `Window basics` **done** (concepts `[window function]`), milestone 1 `Frames` (concepts `[window frame]`). One struggle-repair item `window function`/sql, `hands-on`, base 82. **Energy `low`** (capability 3/10). | **`window function`** (hands-on, score 80, `plan_refs=[(sql-windows, None)]`); no alternates; `energy_deferred=[(sql-windows, milestone 1, floor 5, capability 3)]`; JSON gains `energy_deferred`. | Rule 3: capability 3 < floor 5, so the *new* milestone (Frames) is deferred and named, not synthesised; the plan-related repair on a finished milestone's concept stays eligible and keeps its ref (`None`: plan-related repair, not the next milestone). Score = 82 + 12 bias − 14 (hands-on at low energy). | **no** — owner, 2026-09-16: a struggle-repair task has no energy demand of its own; recommending hands-on repair of a *live* struggle on a low-energy day risks compounding the struggle and damaging confidence (RSD). Finding for council: (1) derive a per-item energy demand for repair from struggle recency/teach-back — at low energy a live struggle defers like new work, a recovered one stays eligible as gentle review; (2) when nothing plan-related fits the day's capability, synthesise a body-doubling / open-session candidate (feature exists: ADR-0001/0003, `web/routes/body_double.py`) naming the deferred items, instead of the least-bad task. | | 4 | Fully-checked | Plan `done-plan` ("Done Plan"), milestones A and B both done. One due item `decorators`/python base 100. | **`decorators`** (118, no refs); `completion_actions=[(done-plan, "Every milestone of 'Done Plan' is checked off — close the plan or extend it with a follow-on mission.")]`; no `study_plan:` candidate anywhere; JSON gains `active_plans` + `completion_actions`. | Rule 9: a fully-checked plan is reported as a completion action and is neither matched (no bias, no refs) nor synthesised. | **primary yes / completion action no as phrased** — owner, 2026-09-16: the completion action must be contextual and consensual. Run the end assessment (`assess(phase="end")`: due reviews, struggles, unverified milestones on the plan's concepts). If outstanding work touches the plan's concepts (or their prerequisites — F2 concept edges), propose *extend* and name the evidence; if clean, propose *close* and ask the learner to agree ("anything you are not comfortable with?"). Status never changes automatically (#7). Natural vehicle: architect with `purpose=planning` and the assessment in the brief (`plan close `, sibling of `plan repair `). Finding for council. | -| 4b | Fully-checked — **re-run after D-G (item 4, 2026-09-18)** | Row 4's fixture (`done-plan`, milestones A `[alpha]` and B `[beta]` both done; one due item `decorators`/python base 100), plus the end assessment's readers planted: **(a)** one due review on plan concept `alpha` (`overdue`) with session mentions backing both concepts; **(b)** no due rows, same mentions. | Primary unchanged in both: **`decorators`** (118, no refs); no `study_plan:` candidate. **(a)** `completion_actions=[(done-plan, due 1 / struggles 0 / unverified 0, proposal **extend**, evidence `["Due review: alpha — overdue"]`)]`, sentence: "Every milestone of 'Done Plan' is checked off, and the closing review proposes extending the plan — 1 due review, 0 struggles and 0 unverified milestones on its concepts. Walk the evidence with the architect: studyloop plan close done-plan." **(b)** counts 0/0/0, proposal **close**, evidence `[]`, sentence: "Every milestone of 'Done Plan' is checked off and the closing review is clean — it proposes closing the plan. Close it with the architect when you agree: studyloop plan close done-plan." No warnings; JSON gains the five keys only inside the entry. | Rule 9 as before for the ranking. The completion action is now the end assessment read as a preview (`assess(phase="end", record=False)`, one call, no write, no status change): `extend` iff any of the three counts on the plan's own concepts is above zero, else `close`; due counts only rows naming a concept (the scheduler's "new topic" row is excluded — owner decision 2026-09-17). `plan close done-plan` launches the architect with the same review as the brief's first section; status moves only when the learner agrees. | **PENDING** — owner: does (a) read as a proposal you would walk, and (b) as a close you would agree to? | +| 4b | Fully-checked — **re-run after D-G (item 4, 2026-09-18)** | Row 4's fixture (`done-plan`, milestones A `[alpha]` and B `[beta]` both done; one due item `decorators`/python base 100), plus the end assessment's readers planted: **(a)** one due review on plan concept `alpha` (`overdue`) with session mentions backing both concepts; **(b)** no due rows, same mentions. | Primary unchanged in both: **`decorators`** (118, no refs); no `study_plan:` candidate. **(a)** `completion_actions=[(done-plan, due 1 / struggles 0 / unverified 0, proposal **extend**, evidence `["Due review: alpha — overdue"]`)]`, sentence: "Every milestone of 'Done Plan' is checked off, and the closing review proposes extending the plan — 1 due review, 0 struggles and 0 unverified milestones on its concepts. Walk the evidence with the architect: studyloop plan close done-plan." **(b)** counts 0/0/0, proposal **close**, evidence `[]`, sentence: "Every milestone of 'Done Plan' is checked off and the closing review is clean — it proposes closing the plan. Close it with the architect when you agree: studyloop plan close done-plan." No warnings; JSON gains the five keys only inside the entry. | Rule 9 as before for the ranking. The completion action is now the end assessment read as a preview (`assess(phase="end", record=False)`, one call, no write, no status change): `extend` iff any of the three counts on the plan's own concepts is above zero, else `close`; due counts only rows naming a concept (the scheduler's "new topic" row is excluded — owner decision 2026-09-17). `plan close done-plan` launches the architect with the same review as the brief's first section; status moves only when the learner agrees. | **yes / yes** — owner, 2026-09-18, answering the two questions as posed: (a) **yes**, a proposal the owner would walk; (b) **yes**, a close the owner would agree to. No further line given. Closes row 4's "no as phrased" finding; status still moves only when the learner agrees in the architect conversation. | | 5 | No-plan identical | No plan documents at all; no collector candidates. | **`one tiny recall loop`** (python, recall, 28, `source=starter`) — the starter. | D-5: `serialise(plan) == golden` → **byte-identical** (`True` in the run); the JSON key list is exactly the golden's — no additive key is present. | **verified** — owner walkthrough 2026-09-16: nothing to judge; the byte-identical golden is the acceptance. | ## How to re-run diff --git a/docs/study-plans.md b/docs/study-plans.md index e08902218..2fcab0a35 100644 --- a/docs/study-plans.md +++ b/docs/study-plans.md @@ -251,7 +251,8 @@ energy-deferred scenario (hands-on repair of a live struggle on a low-energy day) and the completion action's wording were not. The energy-deferred scenario is still follow-on work rather than an edit to the ranking; the completion action was reworked into the closing review described above, and -its re-run row awaits the maintainer's score. +its re-run row was scored by the maintainer on 2026-09-18 — accepted on both +readings, the evidence-backed *extend* and the clean *close*. ## Deliberately not automatic diff --git a/openspec/changes/plan-integration-followons/tasks.md b/openspec/changes/plan-integration-followons/tasks.md index 53a2267a5..8549ae682 100644 --- a/openspec/changes/plan-integration-followons/tasks.md +++ b/openspec/changes/plan-integration-followons/tasks.md @@ -146,7 +146,8 @@ writer, through the existing gate, closes that. Kept out of item 3 so item 3's f - [x] **T4.3** (in `82293293`: row **4b** added under row 4 — row 4 untouched — with the re-run primary (`decorators` 118, unchanged) and both readings of the completion action as the engine emitted them through the RED tests' own fixtures: (a) one due review on plan concept `alpha` → `extend`, evidence - `Due review: alpha — overdue`; (b) clean → `close`, evidence empty. Verdict `PENDING` for the owner; the + `Due review: alpha — overdue`; (b) clean → `close`, evidence empty. Verdict scored by the owner on + 2026-09-18: **yes** on both readings — the scenario-4 "no as phrased" finding is closed; the receipt's status line and re-run mapping name the two backing tests.) Rubric: add row **4b** to `receipts/now-rubric-2026-09-16.md` (do not overwrite row 4) with the re-run primary and the proposal; verdict column `PENDING` for the owner. @@ -174,7 +175,8 @@ writer, through the existing gate, closes that. Kept out of item 3 so item 3's f golden byte-identity still green. - [ ] **T5.3** GREEN: `energy_demand`, rule 3 extension, `body_double` synthesis, CLI/Today "sit with the plan" line. - [ ] ⚖ **T5.4** Council review 7 (`review7`, same seats or `kimi-k2-thinking` third); arbitration; corrections. -- [ ] **T5.5** Rubric row **3b** for the owner (`PENDING`); the change is archived only after the owner scores 3b and 4b. +- [ ] **T5.5** Rubric row **3b** for the owner (`PENDING`); the change is archived only after the owner scores 3b + (4b was scored **yes / yes** on 2026-09-18, so 3b is the one row still outstanding). ## Item 6 — proposals (no code) From 3844c3e05d37496a4894552815e5d55c37bc0be2 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 10:52:56 +0100 Subject: [PATCH 06/23] =?UTF-8?q?test(verify):=20RED=20=E2=80=94=20registe?= =?UTF-8?q?red=20checks=20for=20the=20architect=20grants=20and=20the=20rep?= =?UTF-8?q?air/close=20refusals;=20a=20red=20suite=20names=20its=20nodes?= =?UTF-8?q?=20(T6.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design §6 of plan-integration-followons asks the verification receipt for two more registered checks beside the golden: the two architect grants (the nine plan tools + record_plan_learning, derived from the inventory, in the spelling each harness honours) and the plan repair / plan close refusal texts as the CLI seam tests pin them. Five tests pin their names, shapes and one negative case (a copy of the tree with one grant removed and one stray grant added must fail naming both). The fifth test is receipt honesty: the first real verify run on 9d10fee6 recorded full-suite-studyloop as ok:false with a 12-line output_tail, so the receipt could not say WHICH of the sandbox's 30 failures + 14 errors occurred and could not be reconciled against the 44 named environmental ids. A failed pytest check now keeps every FAILED/ERROR short-summary line on its row. RED: 5 failed / 21 passed, each for the intended missing name, function or key. --- .../test_verify_plan_integration_script.py | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/packages/studyloop/tests/test_verify_plan_integration_script.py b/packages/studyloop/tests/test_verify_plan_integration_script.py index b46472f37..5509059da 100644 --- a/packages/studyloop/tests/test_verify_plan_integration_script.py +++ b/packages/studyloop/tests/test_verify_plan_integration_script.py @@ -78,6 +78,11 @@ def script(): "js-unit", "openspec-validate", "mkdocs-strict", + # Follow-on programme, design §6 (items 1 and 3/4): the two architect grants + # derived from the inventory, and the `plan repair` / `plan close` refusal + # texts as the seam tests pin them. + "architect-grants", + "repair-close-refusals", } @@ -158,6 +163,79 @@ def test_protected_file_checks_name_the_ten_files_against_their_bases(self, scri for rel in (*early_files, *late_files): assert (REPO_ROOT / rel).exists(), rel + def test_architect_grants_check_derives_the_ten_names_from_the_inventory(self, script) -> None: + """Design §6 (follow-on item 1, D-A): the Kiro and Claude architect + definitions carry exactly the nine plan tools + ``record_plan_learning`` + in the spelling each harness honours (probe receipt 2026-09-16/17). + The check is in-process — it reads the two files and the inventory, so a + tenth plan tool added to the inventory fails the receipt until the grants + follow — and it measures the names it found, so the receipt can be read.""" + by_name = {check.name: check for check in script.build_checks(REPO_ROOT)} + check = by_name["architect-grants"] + assert callable(check.command) + assert check.command is script.check_architect_grants + code, measured = script.check_architect_grants(REPO_ROOT) + assert code == 0, measured + from studyloop.mcp.inventory import LEARNING_RECORD_TOOL, PLAN_TOOL_NAMES + + expected = [*PLAN_TOOL_NAMES, LEARNING_RECORD_TOOL] + assert measured["expected"] == expected + assert measured["kiro"]["granted"] == expected + assert measured["claude"]["granted"] == expected + assert measured["kiro"]["file"] == "agents/kiro/study-plan-architect.json" + assert measured["claude"]["file"] == "agents/claude/study-plan-architect.md" + assert measured["problems"] == [] + + def test_architect_grants_check_fails_when_a_grant_is_missing_or_extra( + self, script, tmp_path: Path + ) -> None: + """The check judges a tree, not the checkout: a copy with one grant + removed and one stray studyloop grant added fails, naming both.""" + import json as _json + import shutil + + root = tmp_path / "tree" + (root / "agents/kiro").mkdir(parents=True) + (root / "agents/claude").mkdir(parents=True) + shutil.copy( + REPO_ROOT / "agents/claude/study-plan-architect.md", + root / "agents/claude/study-plan-architect.md", + ) + kiro = _json.loads((REPO_ROOT / "agents/kiro/study-plan-architect.json").read_text()) + allowed = [ + entry for entry in kiro["allowedTools"] if entry != "@studyloop/delete_study_plan" + ] + allowed.append("@studyloop/get_next_action") + kiro["allowedTools"] = allowed + (root / "agents/kiro/study-plan-architect.json").write_text(_json.dumps(kiro)) + code, measured = script.check_architect_grants(root) + assert code == 1 + joined = " ".join(measured["problems"]) + assert "delete_study_plan" in joined and "get_next_action" in joined + assert "kiro" in joined.lower() + + def test_repair_close_refusals_check_names_the_seam_tests_that_pin_the_texts( + self, script + ) -> None: + """Design §6 (items 3/4, D-C/D-G): the refusal texts are pinned by the + CLI seam tests, so the check runs exactly those node ids — the husk + refusal (both exits named), `plan repair` on a ready plan and on an + unknown id, `plan close` on an unfinished plan — and nothing else.""" + by_name = {check.name: check for check in script.build_checks(REPO_ROOT)} + command = by_name["repair-close-refusals"].command + assert not callable(command) + node_ids = [part for part in command if "::" in part] + assert node_ids == [ + "packages/studyloop/tests/test_cli_plan_seam.py::test_husk_refusal_names_both_pause_and_repair", + "packages/studyloop/tests/test_cli_plan_seam.py::test_plan_repair_on_a_ready_plan_says_nothing_to_repair", + "packages/studyloop/tests/test_cli_plan_seam.py::test_plan_repair_unknown_id_is_the_seams_not_found", + "packages/studyloop/tests/test_cli_plan_seam.py::test_plan_close_on_an_unfinished_plan_refuses", + ] + source = (REPO_ROOT / "packages/studyloop/tests/test_cli_plan_seam.py").read_text() + for node in node_ids: + assert f"def {node.split('::')[1]}(" in source, node + assert by_name["repair-close-refusals"].expected_exit == 0 + class TestPytestCounts: @pytest.mark.parametrize( @@ -262,6 +340,45 @@ def test_one_failed_check_fails_the_receipt_and_the_process( assert row["counts"] == {"failed": 1, "passed": 29} assert receipt["summary"]["failed"] == 1 + def test_a_failed_pytest_check_names_its_failed_nodes(self, script, tmp_path: Path) -> None: + """A red full suite is only auditable if the receipt says WHICH tests + failed: the 12-line ``output_tail`` cannot hold 44 ids, so the first + real run on ``9d10fee6`` recorded ``ok: false`` with no way to + reconcile it against the named environmental set. Every ``FAILED`` / + ``ERROR`` short-summary line is kept, in order, on the row.""" + checks = script.build_checks(REPO_ROOT) + out = tmp_path / "verify-2222222.json" + output = "\n".join( + [ + "F.E. [100%]", + "=================================== ERRORS ===================================", + "___ ERROR at setup of test_b ___", + "E RuntimeError: world", + "================================== FAILURES ==================================", + "___ test_a ___", + "E assert 1 == 2", + "=========================== short test summary info ============================", + "FAILED packages/studyloop/tests/test_x.py::test_a - assert 1 == 2", + "ERROR packages/studyloop/tests/test_y.py::test_b - RuntimeError: world", + "1 failed, 2 passed, 1 error in 0.30s", + ] + ) + status = script.run_and_write( + checks, + out=out, + runner=_fake_runner({"full-suite-studyloop": (1, output)}), + tree={"sha": "2222222", "dirty": False}, + ) + assert status == 1 + receipt = json.loads(out.read_text(encoding="utf-8")) + row = next(r for r in receipt["checks"] if r["name"] == "full-suite-studyloop") + assert row["failed_nodes"] == [ + "FAILED packages/studyloop/tests/test_x.py::test_a", + "ERROR packages/studyloop/tests/test_y.py::test_b", + ] + green = next(r for r in receipt["checks"] if r["name"] == "architecture-guard") + assert green["failed_nodes"] == [] + def test_a_check_that_cannot_run_is_a_failure_not_not_applicable( self, script, tmp_path: Path ) -> None: From b977300762264107426145e526fbecbbfa371ce4 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 10:55:12 +0100 Subject: [PATCH 07/23] =?UTF-8?q?feat(verify):=20register=20the=20architec?= =?UTF-8?q?t-grants=20and=20repair-close-refusals=20checks;=20a=20red=20py?= =?UTF-8?q?test=20check=20names=20its=20failed=20nodes=20(T6.3)=20?= =?UTF-8?q?=E2=80=94=20GREEN?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design §6 of plan-integration-followons: two registered checks beside the golden. `architect-grants` is in-process — it derives the ten names from studyloop.mcp.inventory (PLAN_TOOL_NAMES + LEARNING_RECORD_TOOL) and judges agents/kiro/study-plan-architect.json (`@studyloop` visible in `tools`; allowedTools carries exactly the ten as `@studyloop/`, no bare server grant, no inert mcp_studyloop_* spelling) and the Claude architect's `tools:` frontmatter (exactly the ten as `mcp__studyloop__`, no other server), measuring what it found so the receipt reads without the files. `repair-close-refusals` runs exactly the four CLI seam node ids that pin the husk refusal (both exits), `plan repair` on a ready plan and an unknown id, and `plan close` on an unfinished plan. Receipt honesty: a failed pytest check now keeps every FAILED/ERROR short-summary node id on its row (`failed_nodes`), so a red full suite can be reconciled against the named environmental set from the receipt alone — the first real run on 9d10fee6 could not be. Registry 29 -> 31. Verify-script tests 5 RED -> 26 passed; both new checks pass on this tree; ruff, format, pyright clean. --- scripts/verify/plan_integration.py | 138 ++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 2 deletions(-) diff --git a/scripts/verify/plan_integration.py b/scripts/verify/plan_integration.py index 43db66724..aff7940bd 100644 --- a/scripts/verify/plan_integration.py +++ b/scripts/verify/plan_integration.py @@ -7,8 +7,10 @@ protected files against their two bases, the ``rg`` invariants, the combined Web+MCP journey alone and inside the ``-m integration`` run, the #14 browser module under ``-m e2e``, the JS unit tests, ``openspec validate`` and ``mkdocs ---strict`` — and writes one JSON receipt with every command, exit status, -pytest node count and measured value: +--strict``, and (follow-on programme, design §6) the two architect grants +derived from the inventory and the ``plan repair`` / ``plan close`` refusal +texts — and writes one JSON receipt with every command, exit status, pytest +node count, measured value and, for a red pytest check, every failed node id: docs/architecture/plan-integration/receipts/verify-.json @@ -168,6 +170,106 @@ def check_inventory_in_process(repo_root: Path) -> tuple[int, dict[str, Any]]: } +#: The two harness-launched architects that D-A grants the plan tools to, with +#: the grant spelling each harness honours (probe receipt 2026-09-16, re-run on +#: kiro-cli 2.22.0 on 2026-09-17): Kiro reads ``allowedTools`` as +#: ``@/``; Claude Code's ``tools:`` frontmatter as ``mcp____``. +KIRO_ARCHITECT = "agents/kiro/study-plan-architect.json" +CLAUDE_ARCHITECT = "agents/claude/study-plan-architect.md" + + +def _claude_frontmatter_tools(text: str) -> list[str]: + """The comma-separated ``tools:`` allow-list of a Claude subagent file.""" + lines = text.splitlines() + if not lines or lines[0].strip() != "---": + return [] + for line in lines[1:]: + if line.strip() == "---": + break + if line.startswith("tools:"): + return [item.strip() for item in line.removeprefix("tools:").split(",") if item.strip()] + return [] + + +def check_architect_grants(repo_root: Path) -> tuple[int, dict[str, Any]]: + """Design §6 (follow-on item 1, D-A): the Kiro and Claude architect + definitions grant exactly the nine plan tools + ``record_plan_learning`` + from the ``studyloop`` server — derived from the inventory, so a tenth plan + tool fails this check until the grants follow — in the spelling each + harness honours, and nothing else from that server. Kiro must also make + the server *visible* (``@studyloop`` in ``tools``); visibility and trust + are two arrays there.""" + expected = [*PLAN_TOOL_NAMES, LEARNING_RECORD_TOOL] + problems: list[str] = [] + + kiro_path = repo_root / KIRO_ARCHITECT + kiro_granted: list[str] = [] + kiro_visible = False + if not kiro_path.exists(): + problems.append(f"kiro: {KIRO_ARCHITECT} missing") + else: + try: + definition = json.loads(kiro_path.read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + problems.append(f"kiro: {KIRO_ARCHITECT} is not JSON: {exc}") + definition = {} + kiro_visible = "@studyloop" in definition.get("tools", []) + if not kiro_visible: + problems.append("kiro: '@studyloop' absent from tools — the server is invisible") + allowed = definition.get("allowedTools", []) + kiro_granted = [ + entry.removeprefix("@studyloop/") + for entry in allowed + if entry.startswith("@studyloop/") + ] + if "@studyloop" in allowed: + problems.append("kiro: bare '@studyloop' in allowedTools trusts the whole server") + inert = [entry for entry in allowed if entry.startswith("mcp_studyloop_")] + if inert: + problems.append(f"kiro: inert mcp_studyloop_* spelling in allowedTools: {inert}") + _compare_grants("kiro", kiro_granted, expected, problems) + + claude_path = repo_root / CLAUDE_ARCHITECT + claude_granted: list[str] = [] + if not claude_path.exists(): + problems.append(f"claude: {CLAUDE_ARCHITECT} missing") + else: + tools = _claude_frontmatter_tools(claude_path.read_text(encoding="utf-8")) + claude_granted = [ + entry.removeprefix("mcp__studyloop__") + for entry in tools + if entry.startswith("mcp__studyloop__") + ] + other_mcp = [ + entry + for entry in tools + if entry.startswith("mcp__") and not entry.startswith("mcp__studyloop__") + ] + if other_mcp: + problems.append(f"claude: MCP tools from another server: {other_mcp}") + _compare_grants("claude", claude_granted, expected, problems) + + return (1 if problems else 0), { + "expected": expected, + "kiro": {"file": KIRO_ARCHITECT, "visible": kiro_visible, "granted": kiro_granted}, + "claude": {"file": CLAUDE_ARCHITECT, "granted": claude_granted}, + "problems": problems, + } + + +def _compare_grants( + harness: str, granted: list[str], expected: list[str], problems: list[str] +) -> None: + missing = [name for name in expected if name not in granted] + extra = [name for name in granted if name not in expected] + if missing: + problems.append(f"{harness}: plan tools not granted: {missing}") + if extra: + problems.append(f"{harness}: studyloop tools granted beyond the ten: {extra}") + if len(granted) != len(set(granted)): + problems.append(f"{harness}: duplicate grants: {granted}") + + def build_checks(repo_root: Path) -> list[Check]: """The registry. Order is the order the receipt reports and the run executes.""" js_tests = sorted( @@ -210,6 +312,8 @@ def build_checks(repo_root: Path) -> list[Check]: _pytest(f"{TESTS}/test_mcp_stdio_smoke.py", "-m", "integration"), ), Check("inventory-in-process", check_inventory_in_process), + # --- the harness grants derived from that inventory (follow-on D-A) --- + Check("architect-grants", check_architect_grants), # --- the named plan suites (review 4, T6.2 list) ---------------------- Check( "plan-suites", @@ -234,6 +338,20 @@ def build_checks(repo_root: Path) -> list[Check]: ), ), Check("docs-contract", _pytest(f"{TESTS}/test_docs_plan_integration_contract.py")), + # --- the repair / close refusal texts, as the seam tests pin them ----- + # (follow-on D-C / D-G, design §6): the husk refusal naming both exits, + # `plan repair` on a ready plan and on an unknown id, `plan close` on an + # unfinished plan. Exactly these node ids, so a reworded refusal that + # the seam tests still accept is not silently blessed by a wider run. + Check( + "repair-close-refusals", + _pytest( + f"{TESTS}/test_cli_plan_seam.py::test_husk_refusal_names_both_pause_and_repair", + f"{TESTS}/test_cli_plan_seam.py::test_plan_repair_on_a_ready_plan_says_nothing_to_repair", + f"{TESTS}/test_cli_plan_seam.py::test_plan_repair_unknown_id_is_the_seams_not_found", + f"{TESTS}/test_cli_plan_seam.py::test_plan_close_on_an_unfinished_plan_refuses", + ), + ), # --- protected files: byte-identical to their bases ------------------- Check( "protected-files-3a4f6b01", @@ -373,6 +491,21 @@ def _tail(output: str, lines: int = 12) -> list[str]: return [line for line in output.splitlines() if line.strip()][-lines:] +#: pytest's short-summary lines (``-r`` is on under the packages' config): the +#: node id, without the ``- `` suffix, is what a receipt reader needs +#: to reconcile a red suite against the named environmental set. +_FAILED_NODE = re.compile(r"^(?PFAILED|ERROR) (?P\S+)") + + +def _failed_nodes(output: str) -> list[str]: + nodes: list[str] = [] + for line in output.splitlines(): + match = _FAILED_NODE.match(line) + if match: + nodes.append(f"{match['outcome']} {match['node']}") + return nodes + + def run_and_write( checks: Sequence[Check], *, @@ -407,6 +540,7 @@ def run_and_write( "measured": measured, "error": error, "output_tail": [] if ok else _tail(output), + "failed_nodes": [] if ok or callable(check.command) else _failed_nodes(output), } rows.append(row) status = " ok " if ok else "FAIL" From f937b1b5220dc61e9291319501ae22c8db80d324 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 11:14:18 +0100 Subject: [PATCH 08/23] fix(plan): a partial end assessment never proposes a clean close (council review 6, F1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit evaluate_plan turns a history reader that fails into one warning (' unavailable — evaluation is partial') and an empty default, so a count read while that reader was down is unread, not zero. CompletionReview read those zeros as a clean slate: the now engine said 'the closing review is clean — it proposes closing the plan' and plan close printed 'proposes: close' with the gap relegated to a separate section. GPT-Astra 🔴 F1, Grok 🔵 (v); reproduced with two RED tests before this change. The fix lives in the one definition. CompletionReview keys on the evaluator's own marker (PARTIAL_READ_MARKER, defined once in evaluation.py and used by _safe), keeps the counts it did read, sets partial=True, proposes None — neither close nor extend — and names each gap among its evidence lines ('Not read: …'), so both surfaces carry it without a second path. CompletionAction gains partial; the sentence has a third branch that says the review is partial and could not propose; plan close's proposal line reads 'unassessed — the review is partial' and its status line no longer says the review proposes. The separate '### Data gaps' brief section is gone: the gaps are the review's own lines now. Persona 'Closing a Plan' tells the architect what an unassessed proposal means (walk what was read, prefer re-running the review, never infer a clean slate); three projections re-projected, manifest regenerated (updated restored on the 20 unmoved entries), baseline refreshed whole-repo with the pinned detect-secrets 1.5.0 (exactly the two manifest hashes, 72 -> 72 files). Spec delta states the rule and adds the scenario. 66/66 across the two item-4 files (2 RED -> green), 614 across the plan suites + pins, JS 136/136, golden sha unchanged, ruff/pyright/mkdocs/openspec clean. --- .secrets.baseline | 6 +- agents/claude/study-plan-architect.md | 13 ++++- agents/kiro/study-plan-architect/persona.md | 13 ++++- agents/manifest.json | 8 +-- agents/opencode/study-plan-architect.md | 13 ++++- agents/shared/personas/plan-architect.md | 13 ++++- .../specs/active-learning-decisions/spec.md | 24 ++++++++ packages/studyloop/src/studyloop/cli/_plan.py | 35 +++++++----- .../src/studyloop/learning/decision.py | 25 ++++++--- .../src/studyloop/planning/evaluation.py | 7 ++- .../studyloop/src/studyloop/planning/views.py | 23 +++++++- .../studyloop/tests/test_cli_plan_seam.py | 56 +++++++++++++++++++ .../studyloop/tests/test_now_plan_guidance.py | 37 ++++++++++++ 13 files changed, 229 insertions(+), 44 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 4d5cafbde..fab6c5c9c 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -144,7 +144,7 @@ { "type": "Hex High Entropy String", "filename": "agents/manifest.json", - "hashed_secret": "1953bb2b37b175c55c56751ab15fdaa32524b144", + "hashed_secret": "3e0edabd55b9dc4a724219525c5a4ae4011f92fe", "is_verified": false, "line_number": 9 }, @@ -179,7 +179,7 @@ { "type": "Hex High Entropy String", "filename": "agents/manifest.json", - "hashed_secret": "6a33435d0d8b33083d2d7689d9d6c6a8c8b4bc6b", + "hashed_secret": "082e1bcad6af53984a8397dceb67e77caf7da416", "is_verified": false, "line_number": 33 }, @@ -2056,5 +2056,5 @@ } ] }, - "generated_at": "2026-09-17T21:00:18Z" + "generated_at": "2026-09-18T10:12:57Z" } diff --git a/agents/claude/study-plan-architect.md b/agents/claude/study-plan-architect.md index 86e45dae8..45fcfa6ec 100644 --- a/agents/claude/study-plan-architect.md +++ b/agents/claude/study-plan-architect.md @@ -202,7 +202,8 @@ A plan whose every milestone is checked is finished work, not yet a finished plan. `studyloop now` and the Today card report it as a completion action that carries the end assessment on the plan's own concepts — due reviews, struggles, and milestones marked done without evidence — and a proposal: `extend` while any -count is above zero, `close` when all three are zero. `studyloop plan close +count is above zero, `close` when all three are zero, and **no proposal** when +the review is partial. `studyloop plan close PLAN_ID` launches you with a brief whose first section, **Closing review**, lists the three counts, the proposal and one line per counted item, followed by the plan as it stands. The brief's opening line says this is a CLOSING REVIEW @@ -227,8 +228,14 @@ session. The review counts only due rows that name a concept: the scheduler's never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the spaced-repetition loop keeps what the plan taught. -If the brief carries a **Data gaps** section, the counts are partial. Say so -before you propose anything. +If the proposal line reads `unassessed — the review is partial`, one of the +assessment's readers was unavailable and the counts are what was read so far; +the review lists each gap as a `Not read:` line. Say so before anything else, +walk the lines that were read, and do not infer a clean slate from zeros the +review could not fill: propose nothing yourself until the learner has heard +what is missing, and prefer re-running the review (`evaluate_study_plan(plan_id, +"end")`) over closing on a partial one. The same applies when `studyloop now` +or the Today card shows a completion action with no proposal. ## Evaluating a Plan diff --git a/agents/kiro/study-plan-architect/persona.md b/agents/kiro/study-plan-architect/persona.md index ecdf633f4..3d0eb6d5b 100644 --- a/agents/kiro/study-plan-architect/persona.md +++ b/agents/kiro/study-plan-architect/persona.md @@ -196,7 +196,8 @@ A plan whose every milestone is checked is finished work, not yet a finished plan. `studyloop now` and the Today card report it as a completion action that carries the end assessment on the plan's own concepts — due reviews, struggles, and milestones marked done without evidence — and a proposal: `extend` while any -count is above zero, `close` when all three are zero. `studyloop plan close +count is above zero, `close` when all three are zero, and **no proposal** when +the review is partial. `studyloop plan close PLAN_ID` launches you with a brief whose first section, **Closing review**, lists the three counts, the proposal and one line per counted item, followed by the plan as it stands. The brief's opening line says this is a CLOSING REVIEW @@ -221,8 +222,14 @@ session. The review counts only due rows that name a concept: the scheduler's never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the spaced-repetition loop keeps what the plan taught. -If the brief carries a **Data gaps** section, the counts are partial. Say so -before you propose anything. +If the proposal line reads `unassessed — the review is partial`, one of the +assessment's readers was unavailable and the counts are what was read so far; +the review lists each gap as a `Not read:` line. Say so before anything else, +walk the lines that were read, and do not infer a clean slate from zeros the +review could not fill: propose nothing yourself until the learner has heard +what is missing, and prefer re-running the review (`evaluate_study_plan(plan_id, +"end")`) over closing on a partial one. The same applies when `studyloop now` +or the Today card shows a completion action with no proposal. ## Evaluating a Plan diff --git a/agents/manifest.json b/agents/manifest.json index f10e39254..dab808fc3 100644 --- a/agents/manifest.json +++ b/agents/manifest.json @@ -6,8 +6,8 @@ "updated": "2026-09-14" }, "claude/study-plan-architect.md": { - "hash": "30931bca52b881ac", - "updated": "2026-09-17" + "hash": "cd34b8f4844de7e1", + "updated": "2026-09-18" }, "codex/AGENTS.md": { "hash": "7e6c1a0d534b65f7", @@ -30,8 +30,8 @@ "updated": "2026-09-14" }, "opencode/study-plan-architect.md": { - "hash": "54a0b303285cbad6", - "updated": "2026-09-17" + "hash": "7e66f7e1845067a7", + "updated": "2026-09-18" }, "pi/AGENTS.md": { "hash": "03355b0aa919ef6b", diff --git a/agents/opencode/study-plan-architect.md b/agents/opencode/study-plan-architect.md index 11bcbbb4c..4989f7c1f 100644 --- a/agents/opencode/study-plan-architect.md +++ b/agents/opencode/study-plan-architect.md @@ -213,7 +213,8 @@ A plan whose every milestone is checked is finished work, not yet a finished plan. `studyloop now` and the Today card report it as a completion action that carries the end assessment on the plan's own concepts — due reviews, struggles, and milestones marked done without evidence — and a proposal: `extend` while any -count is above zero, `close` when all three are zero. `studyloop plan close +count is above zero, `close` when all three are zero, and **no proposal** when +the review is partial. `studyloop plan close PLAN_ID` launches you with a brief whose first section, **Closing review**, lists the three counts, the proposal and one line per counted item, followed by the plan as it stands. The brief's opening line says this is a CLOSING REVIEW @@ -238,8 +239,14 @@ session. The review counts only due rows that name a concept: the scheduler's never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the spaced-repetition loop keeps what the plan taught. -If the brief carries a **Data gaps** section, the counts are partial. Say so -before you propose anything. +If the proposal line reads `unassessed — the review is partial`, one of the +assessment's readers was unavailable and the counts are what was read so far; +the review lists each gap as a `Not read:` line. Say so before anything else, +walk the lines that were read, and do not infer a clean slate from zeros the +review could not fill: propose nothing yourself until the learner has heard +what is missing, and prefer re-running the review (`evaluate_study_plan(plan_id, +"end")`) over closing on a partial one. The same applies when `studyloop now` +or the Today card shows a completion action with no proposal. ## Evaluating a Plan diff --git a/agents/shared/personas/plan-architect.md b/agents/shared/personas/plan-architect.md index ecdf633f4..3d0eb6d5b 100644 --- a/agents/shared/personas/plan-architect.md +++ b/agents/shared/personas/plan-architect.md @@ -196,7 +196,8 @@ A plan whose every milestone is checked is finished work, not yet a finished plan. `studyloop now` and the Today card report it as a completion action that carries the end assessment on the plan's own concepts — due reviews, struggles, and milestones marked done without evidence — and a proposal: `extend` while any -count is above zero, `close` when all three are zero. `studyloop plan close +count is above zero, `close` when all three are zero, and **no proposal** when +the review is partial. `studyloop plan close PLAN_ID` launches you with a brief whose first section, **Closing review**, lists the three counts, the proposal and one line per counted item, followed by the plan as it stands. The brief's opening line says this is a CLOSING REVIEW @@ -221,8 +222,14 @@ session. The review counts only due rows that name a concept: the scheduler's never recorded (`studyloop progress CONCEPT -t TOPIC -c confident`), so the spaced-repetition loop keeps what the plan taught. -If the brief carries a **Data gaps** section, the counts are partial. Say so -before you propose anything. +If the proposal line reads `unassessed — the review is partial`, one of the +assessment's readers was unavailable and the counts are what was read so far; +the review lists each gap as a `Not read:` line. Say so before anything else, +walk the lines that were read, and do not infer a clean slate from zeros the +review could not fill: propose nothing yourself until the learner has heard +what is missing, and prefer re-running the review (`evaluate_study_plan(plan_id, +"end")`) over closing on a partial one. The same applies when `studyloop now` +or the Today card shows a completion action with no proposal. ## Evaluating a Plan diff --git a/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md b/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md index 0cd41cf21..ce6013fda 100644 --- a/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md +++ b/openspec/changes/plan-integration-followons/specs/active-learning-decisions/spec.md @@ -26,6 +26,19 @@ concept** (owner decision, 2026-09-17): the scheduler's `New topic -- start fresh` row (`concept: None`, `evidence: configured_topic`) is a cold-start hint for "what should I review now", not a lapsed review, and SHALL NOT be counted; `plan evaluate` keeps the row, the exclusion is the completion review's. +`CompletionAction` and `CompletionReview` SHALL carry `partial: bool`. + +**A partial read SHALL NOT propose** (council review 6, F1). `evaluate_plan` +turns a reader that fails into a warning ending `unavailable — evaluation is +partial` (`evaluation.PARTIAL_READ_MARKER`, one definition) and an empty +default, so a count read while that reader was down is unread, not zero. When +the evaluation carries such a warning the review SHALL keep the counts it did +read, set `partial` true, set `proposal` `None` — neither `close` (a clean +slate is a fact about evidence, not its absence) nor `extend` — and name each +gap among its evidence lines (`Not read: unavailable — …`); the +sentence SHALL say the review is partial and could not propose, never "clean"; +the `plan close` brief's proposal line SHALL read `unassessed — the review is +partial` and its status line SHALL NOT say the review proposes. When the assessment fails, the recommendation SHALL NOT fail: the action SHALL keep the plan-static sentence with `proposal` `None`, the counts `0` and @@ -72,3 +85,14 @@ print each evidence line beneath it, and none SHALL re-rank. - **THEN** `completion_actions[0].action` equals the pre-change sentence, `proposal is None`, `warnings` names the plan and the failure, and the primary is still the collected due item + +#### Scenario: A partial assessment never proposes a clean close +- **WHEN** one of the end assessment's history readers raises inside the + evaluation and every other reader finds nothing outstanding +- **THEN** `completion_actions[0]` carries `proposal is None`, + `partial is True`, the counts `(0, 0, 0)`, an evidence line beginning + `Not read:`, a sentence that says the review is partial and never "clean" or + "closing the plan", and `warnings` names the plan and the unavailable reader; + `plan close ` still launches the architect, its brief's fourth line is + `Proposal: unassessed — the review is partial`, the gap is among the first + section's lines, and its status line does not say the review proposes diff --git a/packages/studyloop/src/studyloop/cli/_plan.py b/packages/studyloop/src/studyloop/cli/_plan.py index 46a0f7d27..ec6dc24d4 100644 --- a/packages/studyloop/src/studyloop/cli/_plan.py +++ b/packages/studyloop/src/studyloop/cli/_plan.py @@ -645,9 +645,7 @@ def plan_repair(ctx: click.Context, plan_id: str, agent: str | None) -> None: ) -def _render_closing_brief( - detail: PlanDetail, review: CompletionReview, gaps: tuple[str, ...] -) -> str: +def _render_closing_brief(detail: PlanDetail, review: CompletionReview) -> str: """The brief ``plan close`` hands the architect: the closing review first, then the plan. The first section's first four ``- `` lines are the three counts and the @@ -655,25 +653,25 @@ def _render_closing_brief( the top without parsing prose, as the repair brief's blockers are. The review is the same :class:`~studyloop.planning.CompletionReview` the ``now`` engine puts on its completion action: one definition, two surfaces. - A ``### Data gaps`` section appears only when the evaluation reported a - reader unavailable, so the agent knows the counts are partial. + A partial read (a reader unavailable, council review 6 F1) is that + definition's business too: the proposal line reads ``unassessed — the + review is partial`` and the review's evidence names each reader that was + not read, so the agent knows the counts are what was read so far. """ + proposal = review.proposal or "unassessed — the review is partial" lines = [ f"Due reviews on plan concepts: {review.due_reviews}", f"Struggles on plan concepts: {review.struggles}", f"Unverified milestones: {review.unverified_milestones}", - f"Proposal: {review.proposal}", + f"Proposal: {proposal}", *review.evidence, ] - brief = ( + return ( "### Closing review\n\n" + "\n".join(f"- {line}" for line in lines) + "\n\n" + _render_plan_as_it_stands(detail.summary) ) - if gaps: - brief += "\n### Data gaps\n\n" + "\n".join(f"- {gap}" for gap in gaps) + "\n" - return brief @plan_group.command("close") @@ -723,10 +721,17 @@ def plan_close(ctx: click.Context, plan_id: str, agent: str | None) -> None: from studyloop.cli._study import study - console.print( - f"[green]{s.plan_id!r} ({s.title}) has every milestone checked; the closing review " - f"proposes: {review.proposal}. Launching the architect to decide with you.[/green]" - ) + if review.partial: + console.print( + f"[yellow]{s.plan_id!r} ({s.title}) has every milestone checked, but the closing " + "review is partial — a reader was unavailable, so it does not propose. Launching " + "the architect to walk what was read with you.[/yellow]" + ) + else: + console.print( + f"[green]{s.plan_id!r} ({s.title}) has every milestone checked; the closing review " + f"proposes: {review.proposal}. Launching the architect to decide with you.[/green]" + ) ctx.invoke( study, topic=s.title, @@ -739,7 +744,7 @@ def plan_close(ctx: click.Context, plan_id: str, agent: str | None) -> None: password="", resume=False, end_session=False, - brief=_render_closing_brief(detail, review, result.warnings), + brief=_render_closing_brief(detail, review), brief_intro=CLOSE_BRIEF_INTRO, ) diff --git a/packages/studyloop/src/studyloop/learning/decision.py b/packages/studyloop/src/studyloop/learning/decision.py index 7d80aeccf..37132cbd5 100644 --- a/packages/studyloop/src/studyloop/learning/decision.py +++ b/packages/studyloop/src/studyloop/learning/decision.py @@ -138,9 +138,12 @@ class CompletionAction: on the plan's own concepts and the proposal they imply — read through the preview path, ``assess(AssessPlan(phase="end", record=False))``: no write, no checkpoint, no status change. ``proposal`` is ``None`` when that - assessment failed: the counts are then *unknown*, not zero — ``action`` - falls back to the plan-static sentence and ``NowPlan.warnings`` says why — - so no renderer reads a clean slate or outstanding work into a failure. The + assessment failed outright — the counts are then *unknown*, not zero — + ``action`` falls back to the plan-static sentence and ``NowPlan.warnings`` + says why — and also when it was **partial** (``partial=True``, council + review 6 F1): a reader was down, the counts are what was read so far, and + the sentence says the review could not be completed rather than "clean". + No renderer reads a clean slate or outstanding work into a failure. The engine proposes; the architect asks; the learner decides; ``set_study_plan_status`` is the only door to ``complete``. """ @@ -153,6 +156,7 @@ class CompletionAction: unverified_milestones: int = 0 proposal: Literal["extend", "close"] | None = None evidence: tuple[str, ...] = () + partial: bool = False def to_json_dict(self) -> dict: data = asdict(self) @@ -768,16 +772,22 @@ def _completion_sentence(plan_id: str, title: str, review: CompletionReview) -> def plural(count: int, noun: str) -> str: return f"{count} {noun}{'' if count == 1 else 's'}" + counts = ( + f"{plural(review.due_reviews, 'due review')}, {plural(review.struggles, 'struggle')} and " + f"{plural(review.unverified_milestones, 'unverified milestone')} on its concepts" + ) + if review.partial: + return ( + f"Every milestone of {title!r} is checked off, but the closing review is partial — " + f"one of its readers was unavailable, so it could not propose; read so far: {counts}. " + f"Walk what was read with the architect: studyloop plan close {plan_id}." + ) if review.proposal == "close": return ( f"Every milestone of {title!r} is checked off and the closing review is clean — " "it proposes closing the plan. Close it with the architect when you agree: " f"studyloop plan close {plan_id}." ) - counts = ( - f"{plural(review.due_reviews, 'due review')}, {plural(review.struggles, 'struggle')} and " - f"{plural(review.unverified_milestones, 'unverified milestone')} on its concepts" - ) return ( f"Every milestone of {title!r} is checked off, and the closing review proposes " f"extending the plan — {counts}. Walk the evidence with the architect: " @@ -800,6 +810,7 @@ def _completion_action( unverified_milestones=review.unverified_milestones, proposal=review.proposal, evidence=review.evidence, + partial=review.partial, ) diff --git a/packages/studyloop/src/studyloop/planning/evaluation.py b/packages/studyloop/src/studyloop/planning/evaluation.py index a47a2f440..67a54dec5 100644 --- a/packages/studyloop/src/studyloop/planning/evaluation.py +++ b/packages/studyloop/src/studyloop/planning/evaluation.py @@ -42,6 +42,11 @@ #: Milestones marked done whose concepts carry no confidence evidence. UNVERIFIED_LABEL = "claimed-done-without-evidence" +#: The phrase every reader-failure warning ends with. A consumer that must +#: tell "unread" from "zero" (the completion review, council review 6 F1) +#: keys on this, so the wording lives here and nowhere else. +PARTIAL_READ_MARKER = "unavailable — evaluation is partial" + @dataclass class ConceptEvidence: @@ -149,7 +154,7 @@ def _safe(label: str, fn, default, warnings: list[str]): return fn() except Exception: logger.debug("plan evaluation: %s unavailable", label, exc_info=True) - warnings.append(f"{label} unavailable — evaluation is partial") + warnings.append(f"{label} {PARTIAL_READ_MARKER}") return default diff --git a/packages/studyloop/src/studyloop/planning/views.py b/packages/studyloop/src/studyloop/planning/views.py index aea6e791a..92c99e513 100644 --- a/packages/studyloop/src/studyloop/planning/views.py +++ b/packages/studyloop/src/studyloop/planning/views.py @@ -23,6 +23,7 @@ from typing import TYPE_CHECKING, Any, Literal from .authoring import READINESS_GATE_DATE, readiness +from .evaluation import PARTIAL_READ_MARKER if TYPE_CHECKING: from .evaluation import PlanEvaluation @@ -782,13 +783,24 @@ class CompletionReview: (:func:`~studyloop.planning.evaluation.evaluate_plan` keeps ten due rows and ten struggle rows): a plan with more outstanding work than that reads as ten — still ``extend``. + + **A partial read never proposes** (council review 6, F1). ``evaluate_plan`` + turns a reader that fails into one warning and an empty default, so a + count read while a reader was down is *unread*, not zero. When the + evaluation carries such a warning the review keeps the counts it did read, + sets ``partial`` and proposes ``None`` — neither ``close`` (a clean slate + is a fact about evidence, not about its absence) nor ``extend`` (nothing + outstanding was observed) — and names each gap among its evidence lines, + so the ``now`` sentence and the ``plan close`` brief both say what was + not read. The architect decides what a partial review means. """ due_reviews: int struggles: int unverified_milestones: int - proposal: CompletionProposal + proposal: CompletionProposal | None evidence: tuple[str, ...] + partial: bool = False @classmethod def from_evaluation(cls, evaluation: PlanEvaluationView) -> CompletionReview: @@ -807,13 +819,20 @@ def from_evaluation(cls, evaluation: PlanEvaluationView) -> CompletionReview: if len(lines) > COMPLETION_EVIDENCE_CAP: more = len(lines) - COMPLETION_EVIDENCE_CAP lines = [*lines[:COMPLETION_EVIDENCE_CAP], f"… and {more} more"] + gaps = [w for w in evaluation.warnings if PARTIAL_READ_MARKER in w] + lines.extend(f"Not read: {gap}" for gap in gaps) counts = (len(due), len(evaluation.struggles), len(evaluation.unverified_milestones)) + # Unread beats zero: a gap yields no proposal at all. + proposal: CompletionProposal | None = ( + None if gaps else ("extend" if any(counts) else "close") + ) return cls( due_reviews=counts[0], struggles=counts[1], unverified_milestones=counts[2], - proposal="extend" if any(counts) else "close", + proposal=proposal, evidence=tuple(lines), + partial=bool(gaps), ) def to_json_dict(self) -> dict[str, Any]: diff --git a/packages/studyloop/tests/test_cli_plan_seam.py b/packages/studyloop/tests/test_cli_plan_seam.py index ecf57d0cc..704b63fe9 100644 --- a/packages/studyloop/tests/test_cli_plan_seam.py +++ b/packages/studyloop/tests/test_cli_plan_seam.py @@ -832,3 +832,59 @@ def test_plan_close_on_an_unfinished_plan_refuses( assert "Traceback" not in clean assert calls == [] # no launch assert _documents(isolated_plans_dir) == before + + +def test_plan_close_with_a_partial_assessment_does_not_present_a_clean_proposal( + runner, isolated_plans_dir, tmp_path, monkeypatch +) -> None: + """Council review 6, F1: one of the end assessment's readers is down, the + rest is clean. ``evaluate_plan`` swallows the failure into a warning and + an empty default, so the counts it did read are zero — but "zero due" is + unread, not known. The brief must say so in its fixed lines + (``Proposal: unassessed — the review is partial``), name the gap in the + same first section, and still launch: the architect is the right place to + decide what a partial review means. The status line must not say the + review "proposes: close".""" + from contextlib import ExitStack + + from studyloop import history + + store.plans_dir() + runner.invoke(cli, ["plan", "new", "--title", "Glue ETL", *READY, "--activate"]) + runner.invoke(cli, ["plan", "milestone", "glue-etl", "0", "--done"]) + runner.invoke(cli, ["plan", "milestone", "glue-etl", "1", "--done"]) + before = _documents(isolated_plans_dir) + _plant_end_evidence( + monkeypatch, + due=[], + mentions=[{"snippet": "walked through the glue job anatomy and a dynamicframe transform"}], + ) + + def due_reader_down(topic_keywords_map): + raise RuntimeError("study_progress is locked") + + monkeypatch.setattr(history, "spaced_repetition_due", due_reader_down) + + captured: dict = {} + calls: list = [] + with ExitStack() as stack: + for p in _launch_patches(tmp_path, captured, calls): + stack.enter_context(p) + monkeypatch.setenv("TMUX", "/tmp/tmux") + result = runner.invoke(cli, ["plan", "close", "glue-etl"]) + + assert result.exit_code == 0, result.output + assert calls == ["Glue ETL"], calls # the launch still happens: the architect decides + clean = _ANSI.sub("", result.output) + assert "proposes: close" not in clean + assert "partial" in clean.lower() + + items = _closing_section(captured["brief"]) + assert items[:4] == [ + "Due reviews on plan concepts: 0", + "Struggles on plan concepts: 0", + "Unverified milestones: 0", + "Proposal: unassessed — the review is partial", + ], items + assert any("unavailable" in item for item in items[4:]), items # the gap, in the same section + assert _documents(isolated_plans_dir) == before diff --git a/packages/studyloop/tests/test_now_plan_guidance.py b/packages/studyloop/tests/test_now_plan_guidance.py index 5fa9a3d63..2c930a3e1 100644 --- a/packages/studyloop/tests/test_now_plan_guidance.py +++ b/packages/studyloop/tests/test_now_plan_guidance.py @@ -1052,3 +1052,40 @@ def boom(self, intent): "done-plan" in warning and "assess" in warning.lower() for warning in plan.warnings ), plan.warnings assert plan.primary.concept == "decorators" + + +def test_completion_partial_assessment_never_proposes_a_clean_close(monkeypatch) -> None: + """Council review 6, F1 (GPT 🔴, Grok 🔵): a reader that fails inside the + evaluation is a *partial* read — ``evaluate_plan`` swallows it into a + warning and an empty default — so "nothing due" is not known, only + unread. The review must carry that: no ``close``, no "clean", the + proposal ``None`` with the counts it did read, and the data gap named on + the plan's warnings. A clean slate is a fact about evidence, never about + its absence.""" + from studyloop import history + + _plan("done-plan", title="Done Plan", topics=["sql"], milestones=_DONE) + _plant_evidence( + monkeypatch, + mentions=[{"snippet": "explained alpha and beta in the teach-back"}], + ) + + def due_reader_down(topic_keywords_map): + raise RuntimeError("study_progress is locked") + + monkeypatch.setattr(history, "spaced_repetition_due", due_reader_down) + _patch_collectors(monkeypatch, _candidate("decorators", topic="python", score=100)) + + plan = build_now_plan() + + [action] = plan.completion_actions + assert action.proposal is None + assert action.partial is True + assert (action.due_reviews, action.struggles, action.unverified_milestones) == (0, 0, 0) + assert "clean" not in action.action.lower() + assert "closing the plan" not in action.action.lower() + assert "partial" in action.action.lower() or "could not" in action.action.lower() + assert "studyloop plan close done-plan" in action.action + assert any("done-plan" in w and "unavailable" in w for w in plan.warnings), plan.warnings + entry = plan.to_json_dict()["completion_actions"][0] + assert entry["proposal"] is None and entry["partial"] is True From 97efcc4aa58215b0f5a7fc685025b6a592085318 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 11:17:57 +0100 Subject: [PATCH 09/23] fix(web): bound the planning launch's wait for the picker's options (council review 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 626ea129 made a planning click await init()'s /api/session/options fetch before judging whether an agent exists, so a cold server no longer refused with 'Select an agent'. That put an unbounded network wait on the click's critical path: a request that never settles (a server that accepts the connection and stalls) held the learner's click forever — no POST, no refusal, no message. qwen3-coder 🔴, GPT-Astra F3, Grok 🔵 (i); reproduced by a JS test that never settles the fetch (timed out at 5 s). The wait now races the options promise against optionsWaitMs (8 s, on the timer's state so tests can shorten it); past the bound the launch judges the agent as it stands and gives the picker's own refusal, and a settlement that arrives later launches nothing on its own. The two existing wait tests (resolve-then-launch once; empty picker still refuses) are unchanged. JS 137/137 (+1), browser journey 11/11 -m e2e; web-ui spec delta states the wait and its bound. --- .../specs/web-ui/spec.md | 8 ++++- .../web/static/js/components/session-timer.js | 19 +++++++++- .../tests/js/plan-architect-launch.test.js | 35 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/openspec/changes/plan-integration-followons/specs/web-ui/spec.md b/openspec/changes/plan-integration-followons/specs/web-ui/spec.md index ebef185c3..97ab16420 100644 --- a/openspec/changes/plan-integration-followons/specs/web-ui/spec.md +++ b/openspec/changes/plan-integration-followons/specs/web-ui/spec.md @@ -24,7 +24,13 @@ the 409 handling and the `study-session-start` event the live console mounts on (`#study-session`), then reports the outcome back with exactly one `plan-architect-result` event. A second activation while a launch is in flight SHALL be a no-op. The Study Session view's `init()` SHALL register its window -listeners once even when called twice (Alpine auto-init plus `x-init`). +listeners once even when called twice (Alpine auto-init plus `x-init`). A +planning launch that arrives before `init()`'s `/api/session/options` fetch has +settled SHALL wait for it before judging whether an agent exists (a cold server +is not a missing agent), and that wait SHALL be bounded (`optionsWaitMs`, 8 s): +past the bound the launch judges the agent as it stands and refuses with the +picker's own `Select an agent to continue.`; a settlement that arrives later +SHALL launch nothing on its own (council review 6). The live console SHALL carry a purpose label (`data-testid="console-purpose-label"`, `role="status"`, `aria-live="polite"`) that is rendered only for a planning diff --git a/packages/studyloop/src/studyloop/web/static/js/components/session-timer.js b/packages/studyloop/src/studyloop/web/static/js/components/session-timer.js index 8717affb2..f71f38dac 100644 --- a/packages/studyloop/src/studyloop/web/static/js/components/session-timer.js +++ b/packages/studyloop/src/studyloop/web/static/js/components/session-timer.js @@ -100,6 +100,12 @@ export function sessionTimer() { studyOptions: { topics: [], vendors: [], courses: [], lessons: [] }, starting: false, startError: '', + /* How long a planning launch will wait for init()'s options fetch before + judging the agent (council review 6). The fetch is on the click's + critical path since 626ea129; a request that never settles must not + hold the click forever — past this bound the launch falls through to + the same "Select an agent" refusal an empty picker gets. Tests shorten it. */ + optionsWaitMs: 8000, /* What the live session is FOR: 'focus' (today's study session) or 'planning' (the study-plan architect, #14 / design §5). Set from the 201 body on a start and from /api/session/state on a restore; it @@ -290,7 +296,18 @@ export function sessionTimer() { (init() sets _optionsReady on every run; a timer whose init never ran has nothing to wait for and falls through to the check.) */ if (purpose === 'planning' && !this.agent && this._optionsReady) { - await this._optionsReady; + /* Bounded (council review 6): a fetch that never settles must not hold + the click. Past the bound the launch judges the agent as it stands; + a settlement that arrives later launches nothing on its own. */ + let bound; + const timedOut = new Promise((resolve) => { + bound = setTimeout(resolve, this.optionsWaitMs); + }); + try { + await Promise.race([this._optionsReady, timedOut]); + } finally { + clearTimeout(bound); + } } if (!this.agent) { /* The Start button is disabled without an agent; a Plans-view launch diff --git a/packages/studyloop/tests/js/plan-architect-launch.test.js b/packages/studyloop/tests/js/plan-architect-launch.test.js index 60bfc4800..a6af3bdf4 100644 --- a/packages/studyloop/tests/js/plan-architect-launch.test.js +++ b/packages/studyloop/tests/js/plan-architect-launch.test.js @@ -500,3 +500,38 @@ test('startPlanning with no agent available after the options resolve still refu assert.equal(posts.length, 0); assert.match(timer.startError, /select an agent/i); }); + +/* Council review 6 (qwen 🔴, GPT F3, Grok 🔵): the wait above put init()'s + * options fetch on the click's critical path with no bound. A request that + * never settles (a server that accepts the connection and stalls) left the + * learner's click awaiting forever — no POST, no refusal, no message. The + * wait is bounded: past `optionsWaitMs` the click falls through to the same + * "Select an agent" refusal it would have given on an empty picker, and a + * later settlement does not launch anything on its own. */ +test('a planning click does not wait forever for options that never settle', async () => { + const baseFetch = globalThis.fetch; + globalThis.fetch = async (url, opts) => { + if (String(url).endsWith('/api/session/options')) { + return new Promise(() => {}); // never settles + } + return baseFetch(url, opts); + }; + const timer = sessionTimer(); + timers.push(timer); + timer.$nextTick = (cb) => cb(); + timer.optionsWaitMs = 50; // the production default is seconds; the bound is what is under test + timer.init(); // never resolves: the options fetch never settles + const seen = startEvents(); + + const started = Date.now(); + const ok = await timer.startPlanning({ topic: 'SQL window functions' }); + const waited = Date.now() - started; + + assert.equal(ok, false); + assert.ok(waited < 2000, `the click must come back within the bound, waited ${waited}ms`); + assert.equal(posts.length, 0, 'nothing was POSTed without an agent'); + assert.match(timer.startError, /select an agent/i); + assert.equal(seen.length, 0); + await settle(); + assert.equal(posts.length, 0, 'no deferred launch fires later'); +}); From 0887b1fb3f104022a309978bc3042c9c8c72831d Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 11:24:14 +0100 Subject: [PATCH 10/23] fix(plan): discovery says only what the seam knows; an unreadable document is named (council review 6, F4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three sentences claimed more than the evidence. husk_provenance read a creation stamp before the gate as 'was never judged by it' — but a plan created before 2026-09-15 can be saved ready after it and hand-edited into a husk later; the stamp establishes when the document was created, not what judged it or when it became incomplete. It now says 'This plan's creation stamp predates the readiness gate (…); the seam cannot tell when it became incomplete.' doctor's healthy row promised 'every write the gate judges will pass' — a future write can remove a required field — and now says 'all ready as they stand.' And husks() skipped a document it could not load, so doctor could report all-ready over a directory holding a file no listing can read. GPT-Astra 🔴 F4; reproduced with a parametrised provenance test and two doctor tests before this change (the first fixture was not unreadable at all — the parser is lenient with malformed front matter — the real class is a non-UTF-8 file or a filename that is not a valid id). PlanApplication.survey_husks() is the one pass that returns both facts (HuskSurvey: husks + unreadable ids); husks() is now a view over it, so plan list/repair are unchanged. doctor emits one warn row per unreadable document beside the readiness rows, fix_auto=False. Health and cli-surface deltas state the honest wording and the new row. 312 across the pinning files, guard 30/30, ruff/format/pyright clean, openspec valid. --- .../specs/cli-surface/spec.md | 6 ++- .../specs/health-and-diagnostics/spec.md | 34 +++++++++++---- .../studyloop/src/studyloop/cli/_doctor.py | 33 ++++++++++---- .../src/studyloop/planning/__init__.py | 2 + .../src/studyloop/planning/application.py | 22 +++++++++- .../studyloop/src/studyloop/planning/views.py | 18 +++++++- packages/studyloop/tests/test_cli_doctor.py | 43 +++++++++++++++++++ .../studyloop/tests/test_plan_application.py | 38 ++++++++++++++++ 8 files changed, 172 insertions(+), 24 deletions(-) diff --git a/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md b/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md index 8608bbd58..e89e48bf9 100644 --- a/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md +++ b/openspec/changes/plan-integration-followons/specs/cli-surface/spec.md @@ -24,8 +24,10 @@ build_canonical_persona`. The brief's first section SHALL be `### Repair: what this plan is missing` listing exactly `readiness.blockers` as `- ` lines and nothing else, followed by the plan as it stands (title, id, status, topics, milestones done/total, created) and one provenance sentence: -`predates the readiness gate` only when `created` parses as a date before -`READINESS_GATE_DATE`; otherwise `cannot tell how it got that way`. The +`creation stamp predates the readiness gate` (and `cannot tell when it became +incomplete`) only when `created` parses as a date before +`READINESS_GATE_DATE`; otherwise `cannot tell how it got that way`; never +`never judged` (council review 6 F4). The sentence SHALL never claim a hand edit. The `brief_intro` SHALL say `PLAN REPAIR` and `ask the learner only for what is missing`; the default intro (`None`) SHALL keep the planning sentence byte-for-byte so the Web door's diff --git a/openspec/changes/plan-integration-followons/specs/health-and-diagnostics/spec.md b/openspec/changes/plan-integration-followons/specs/health-and-diagnostics/spec.md index efb010e79..fc452b11c 100644 --- a/openspec/changes/plan-integration-followons/specs/health-and-diagnostics/spec.md +++ b/openspec/changes/plan-integration-followons/specs/health-and-diagnostics/spec.md @@ -10,14 +10,23 @@ It SHALL emit one `warn` row per husk with `name="study_plans"`, `fix_auto=False` (the repair is a conversation with the architect, not a script), a message naming the plan id, its title, the exact `ReadinessView.blockers`, and the shared provenance sentence -(`husk_provenance`: `predates the readiness gate` only for a `created` before -`READINESS_GATE_DATE`, else `cannot tell how it got that way`, never `hand -edit`), and a `fix_hint` naming both exits: `studyloop plan repair (or: -studyloop plan status paused)`. When every active plan is ready it SHALL -emit one `pass` row counting the active plans; when no plan is active it SHALL -emit one `info` row, not a warning. A draft is unready by nature and is never -reported. A plans directory that cannot be read SHALL be one `warn` row, not -a crash of doctor. +(`husk_provenance`: `This plan's creation stamp predates the readiness gate +(); the seam cannot tell when it became incomplete` only +for a `created` that parses as a date before `READINESS_GATE_DATE`, else +`cannot tell how it got that way`; never `hand edit`, never `never judged` — +a stamp establishes when the document was created, not what judged it or +when it became incomplete; council review 6 F4), and a `fix_hint` naming both +exits: `studyloop plan repair (or: studyloop plan status paused)`. +When every active plan is ready it SHALL emit one `pass` row counting the +active plans and saying they are ready *as they stand* — never a promise +about writes that have not happened (`will pass`, `every write`); when no +plan is active it SHALL emit one `info` row, not a warning. A draft is +unready by nature and is never reported. A plans directory that cannot be +read SHALL be one `warn` row, not a crash of doctor; a single document the +seam cannot read (`PlanApplication.survey_husks().unreadable`: a filename +that is not a valid id, a file that is not UTF-8) SHALL be its own `warn` row +naming the id beside the readiness rows, so a parse failure never reads as +health (F4). #### Scenario: Two husks, one ready active plan, one draft - **WHEN** `check_study_plans()` runs over a ready active plan, a draft with @@ -33,7 +42,14 @@ a crash of doctor. - **WHEN** `check_study_plans()` runs with one ready active plan, and again with no plans at all - **THEN** the first returns one `pass` row saying `1 active plan` and - `ready`; the second returns one `info` row + `ready` and neither `will pass` nor `every write`; the second returns one + `info` row + +#### Scenario: An unreadable document is named, not hidden behind all-ready +- **WHEN** `check_study_plans()` runs over one ready active plan and one + `.md` file the seam cannot read (not UTF-8) +- **THEN** it returns a `warn` row naming the unreadable id (`could not be + read`, `fix_auto=False`) and the `pass` row for the readable plan #### Scenario: The checker is registered - **WHEN** `_get_registry()` is built diff --git a/packages/studyloop/src/studyloop/cli/_doctor.py b/packages/studyloop/src/studyloop/cli/_doctor.py index 62503bde8..a7634d9d6 100644 --- a/packages/studyloop/src/studyloop/cli/_doctor.py +++ b/packages/studyloop/src/studyloop/cli/_doctor.py @@ -79,7 +79,9 @@ def check_study_plans() -> list[CheckResult]: and an honest provenance sentence shared with the ``plan repair`` brief — with ``fix_auto=False``: the repair is a conversation with the architect, not a script. Zero husks among active plans is one ``pass`` row; no plans - at all is ``info``, not a warning. Lives here beside + at all is ``info``, not a warning. A document that cannot be read at all + is its own ``warn`` row naming the file (council review 6, F4): a parse + error must not look like health. Lives here beside ``check_unknown_config_keys`` and joins the same ``config`` category: the health spec enumerates categories verbatim and gains none here. """ @@ -88,7 +90,7 @@ def check_study_plans() -> list[CheckResult]: app = PlanApplication() active = app.browse(status="active") - husks = app.husks() + survey = app.survey_husks() except Exception as exc: # a broken plans dir is a report, not a crash of doctor return [ CheckResult( @@ -101,8 +103,22 @@ def check_study_plans() -> list[CheckResult]: ) ] + rows: list[CheckResult] = [ + CheckResult( + "config", + "study_plans", + "warn", + f"Study plan document '{plan_id}' could not be read, so its readiness is unknown.", + f"Open the file under `studyloop plan list`'s directory and fix its front matter, " + f"or move it out; `studyloop plan show {plan_id}` prints the parse error.", + False, + ) + for plan_id in survey.unreadable + ] + if not active: return [ + *rows, CheckResult( "config", "study_plans", @@ -111,25 +127,24 @@ def check_study_plans() -> list[CheckResult]: "Create one with `studyloop plan architect` when you want a plan to steer " "`studyloop now`.", False, - ) + ), ] - if not husks: + if not survey.husks: n = len(active) return [ + *rows, CheckResult( "config", "study_plans", "pass", - f"{n} active plan{'s' if n != 1 else ''}, all ready — every write the gate " - "judges will pass.", + f"{n} active plan{'s' if n != 1 else ''}, all ready as they stand.", "", False, - ) + ), ] - rows: list[CheckResult] = [] - for husk in husks: + for husk in survey.husks: plan_id = husk.summary.plan_id blockers = "; ".join(husk.readiness.blockers) provenance = husk_provenance(husk.summary.created) diff --git a/packages/studyloop/src/studyloop/planning/__init__.py b/packages/studyloop/src/studyloop/planning/__init__.py index 95c2e6ae5..0a089a7ec 100644 --- a/packages/studyloop/src/studyloop/planning/__init__.py +++ b/packages/studyloop/src/studyloop/planning/__init__.py @@ -99,6 +99,7 @@ CompletionProposal, CompletionReview, DeleteResult, + HuskSurvey, InterviewItemView, LearningRecordOutcome, LearningRecordView, @@ -135,6 +136,7 @@ "DeletePlan", "DeleteResult", "HerdrBackend", + "HuskSurvey", "ImportDocument", "InterviewItemView", "InterviewQuestion", diff --git a/packages/studyloop/src/studyloop/planning/application.py b/packages/studyloop/src/studyloop/planning/application.py index be3e60bec..84091e418 100644 --- a/packages/studyloop/src/studyloop/planning/application.py +++ b/packages/studyloop/src/studyloop/planning/application.py @@ -68,6 +68,7 @@ AssessmentResult, CheckpointHistoryView, DeleteResult, + HuskSurvey, LearningRecordOutcome, LearningRecordView, PlanDetail, @@ -277,19 +278,36 @@ def husks(self) -> tuple[PlanDetail, ...]: ascending ``updated``, ties in id order. A draft with no mission is unready by nature and is not a husk; a paused incomplete plan is what the gate asked for and is not one either. An unreadable document is - logged and skipped, as every listing does. + logged and skipped here, as every listing does — :meth:`survey_husks` + is the read that also names it (council review 6, F4). + """ + return self.survey_husks().husks + + def survey_husks(self) -> HuskSurvey: + """:meth:`husks` plus the ids of the documents that could not be read. + + One pass over the plans directory, two facts: the husks, in + :meth:`husks`' order, and every document ``_load`` refused — a parse + error, a malformed id — so a caller that reports "all ready" can say + so *of the documents it could read* and name the one it could not, + instead of hiding it (council review 6, F4). Read-only. """ found: list[StudyPlan] = [] + unreadable: list[str] = [] for plan_id in store.list_plan_ids(): try: plan = self._load(plan_id) except Exception: # one bad document must not hide the others (as list_plans) logger.warning("Skipping unreadable study plan: %s", plan_id, exc_info=True) + unreadable.append(plan_id) continue if plan.status == "active" and not ReadinessView.from_plan(plan).ready: found.append(plan) found.sort(key=lambda p: p.updated) # stable: id order (list_plan_ids) breaks ties - return tuple(PlanDetail.from_plan(plan) for plan in found) + return HuskSurvey( + husks=tuple(PlanDetail.from_plan(plan) for plan in found), + unreadable=tuple(unreadable), + ) def reindex(self) -> int: """Rebuild the derived SQLite index from the documents. Returns rows written. diff --git a/packages/studyloop/src/studyloop/planning/views.py b/packages/studyloop/src/studyloop/planning/views.py index 92c99e513..f2c0034f9 100644 --- a/packages/studyloop/src/studyloop/planning/views.py +++ b/packages/studyloop/src/studyloop/planning/views.py @@ -135,6 +135,20 @@ def to_json_dict(self) -> dict[str, Any]: } +@dataclass(frozen=True) +class HuskSurvey: + """What one pass over the plans directory found: the husks, and the ids of + the documents that could not be read at all (council review 6, F4). + + ``unreadable`` exists so a surface that says "all ready" can say so of the + documents it read and name the one it could not, rather than let a parse + error look like health. Read-only, like everything in this module. + """ + + husks: tuple[PlanDetail, ...] + unreadable: tuple[str, ...] + + def husk_provenance(created: str) -> str: """One honest sentence on how an active-but-unready plan got that way (item 3). @@ -156,8 +170,8 @@ def husk_provenance(created: str) -> str: predates = False if predates: return ( - f"This plan predates the readiness gate ({READINESS_GATE_DATE}) " - "and was never judged by it." + f"This plan's creation stamp predates the readiness gate ({READINESS_GATE_DATE}); " + "the seam cannot tell when it became incomplete." ) return "This plan is active and incomplete; the seam cannot tell how it got that way." diff --git a/packages/studyloop/tests/test_cli_doctor.py b/packages/studyloop/tests/test_cli_doctor.py index 1c76f9fbf..d27f0a18a 100644 --- a/packages/studyloop/tests/test_cli_doctor.py +++ b/packages/studyloop/tests/test_cli_doctor.py @@ -276,6 +276,49 @@ def test_all_active_plans_ready_is_one_pass_row(self) -> None: assert results[0].category == "config" assert "1 active plan" in results[0].message assert "ready" in results[0].message + # Council review 6, F4: readiness is a verdict on the document as it + # stands, not a promise about writes that have not happened yet. + assert "will pass" not in results[0].message + assert "every write" not in results[0].message + + def test_an_unreadable_plan_document_is_named_not_hidden_behind_all_ready(self) -> None: + """Council review 6, F4: ``husks()`` skips a document it cannot load, + so ``doctor`` could report "all ready" over a directory holding a file + that no listing can read. The learner is told which file, as a + ``warn`` row beside the readiness rows — never silently.""" + from studyloop.cli._doctor import check_study_plans + from studyloop.planning import CreatePlan, PlanApplication + + PlanApplication().apply( + CreatePlan( + title="Ready Active", + plan_id="ready-active", + status="active", + answers={ + "why": "Own the nightly pipeline", + "success": ["Deploy unaided"], + "topics": ["data-engineering"], + "milestones": [{"title": "Job anatomy", "concepts": ["glue job"]}], + }, + ) + ) + # The parser is lenient with malformed front matter (it yields an + # untitled draft), so the unreadable class is a document ``_load`` + # refuses outright: here, a file that is not UTF-8. + (self.plans_dir / "broken.md").write_bytes(b"\xff\xfe\x00not a plan") + + results = check_study_plans() + + statuses = sorted(r.status for r in results) + assert statuses == ["pass", "warn"], results + unreadable = next(r for r in results if r.status == "warn") + assert "broken" in unreadable.message + assert "could not be read" in unreadable.message.lower() or "unreadable" in ( + unreadable.message.lower() + ) + assert unreadable.fix_auto is False + healthy = next(r for r in results if r.status == "pass") + assert "1 active plan" in healthy.message # the readable plan is still judged def test_no_plans_at_all_is_info_not_a_warning(self) -> None: from studyloop.cli._doctor import check_study_plans diff --git a/packages/studyloop/tests/test_plan_application.py b/packages/studyloop/tests/test_plan_application.py index 4db99b483..dd3053bf5 100644 --- a/packages/studyloop/tests/test_plan_application.py +++ b/packages/studyloop/tests/test_plan_application.py @@ -1117,6 +1117,44 @@ def test_revise_partial_mission_on_a_husk_is_refused_and_one_call_repairs_it( assert on_disk.mission.why == "Own the nightly pipeline" +@pytest.mark.parametrize( + ("created", "predates"), + [ + ("2026-09-01T00:00:00+00:00", True), + ("2026-09-14", True), + ("2026-09-15", False), # the gate's own day is not before it + ("2026-09-17T09:00:00+00:00", False), + ("", False), + ("not a date", False), + ], +) +def test_husk_provenance_states_only_what_the_creation_stamp_establishes( + created: str, predates: bool +) -> None: + """Council review 6, F4: a creation stamp before the gate establishes that + the *stamp* predates the gate — not that the plan "was never judged by + it" (a plan created before the gate can be saved ready after it and hand- + edited into a husk later), and never when it became incomplete. The + sentence says the first and disclaims the second; the fallback says the + seam cannot tell. Neither ever claims a hand edit.""" + from studyloop.planning.authoring import READINESS_GATE_DATE + from studyloop.planning.views import husk_provenance + + sentence = husk_provenance(created) + + assert "never judged" not in sentence + assert "hand edit" not in sentence + assert "cannot tell when it became incomplete" in sentence or ( + "cannot tell how it got that way" in sentence + ) + if predates: + assert f"predates the readiness gate ({READINESS_GATE_DATE})" in sentence + assert "creation stamp" in sentence + else: + assert "predates the readiness gate" not in sentence + assert "cannot tell how it got that way" in sentence + + @pytest.mark.parametrize("field", ["success", "constraints", "out_of_scope"]) def test_revise_mission_list_given_a_bare_string_is_invalid_before_any_write( app: PlanApplication, monkeypatch, field: str From 13b5d1211fcf2644affeaaa3843f82498370ba55 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 11:26:18 +0100 Subject: [PATCH 11/23] docs(agents): say where Claude's studyloop server is registered, that mentor grants went live, and when to re-probe kiro-cli (council review 6, F5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Claude architect's frontmatter names ten mcp__studyloop__ tools, and no diff in the reviewed range showed where the server itself is declared for Claude Code — so three seats asked whether the grant was inert the way the mentor's had been (GPT F5 🟡, Grok 🔵 c). It is not: installers._MCP_HARNESSES includes claude and `studyloop install agents` merges both servers into ~/.claude.json's mcpServers. The install doc now says so, and the pin takes the path from installers._mcp_config_path('claude') rather than a remembered string, so the sentence cannot drift from the code. Two more disclosures the seats asked for: existing Kiro study-mentor installs start seeing the twelve MCP tools their file always named once re-installed (f5c2057d fixed the inert spelling; nothing a user read said so — Grok 💡 d, GPT 🔵), and the grant spelling is evidence pinned to kiro-cli 2.21.4/2.22.0, so the doc and the probe receipt's header both say to re-run probes A and B on a newer binary. The receipt also states that the session-db 'visible, prompts' reading is an expectation, not a measurement (Grok 🔵 a). Persona/install/docs/prompt-contract pins 137 passed; mkdocs --strict clean. --- docs/agent-install.md | 20 ++++++++++++++----- .../kiro-agent-tools-probe-2026-09-16.md | 7 +++++++ .../tests/test_plan_architect_persona.py | 14 +++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/docs/agent-install.md b/docs/agent-install.md index 17bf235b6..c547cd9b3 100644 --- a/docs/agent-install.md +++ b/docs/agent-install.md @@ -242,15 +242,25 @@ agent whose `tools` is `@builtin` alone sees no MCP tool, server or not) and trusts exactly the ten tools above as `@studyloop/` in `allowedTools`; the `session-db` tools stay visible but prompt. Claude Code's `agents/claude/study-plan-architect.md` names the same ten in its frontmatter -`tools:` allow-list as `mcp__studyloop__`. That is the least-privilege -grant the maintainer decided on 2026-09-16 (plan-integration follow-on -decision D-A: no harness-launched architect falls back to the shell with -full permissions): nothing else on the `studyloop` server is trusted, and +`tools:` allow-list as `mcp__studyloop__`; the server itself is +registered globally for Claude Code by `studyloop install agents`, which +merges `studyloop` and `session-db` into `~/.claude.json`'s `mcpServers`, so +that allow-list names tools the agent process can actually reach. That is the +least-privilege grant the maintainer decided on 2026-09-16 (plan-integration +follow-on decision D-A: no harness-launched architect falls back to the shell +with full permissions): nothing else on the `studyloop` server is trusted, and the learner's confirmation before `delete_study_plan` remains a persona rule — a tool permission is not the learner's authorisation. One spelling detail matters for Kiro: `@server/tool` is the form an agent config honours; `mcp_server_tool` belongs to `mcp.json`'s `autoApprove` and is ignored in an -agent file. OpenCode, Codex and Grok Build register the server globally +agent file. That spelling was established by probing kiro-cli 2.21.4 and +2.22.0 (`docs/architecture/plan-integration/receipts/kiro-agent-tools-probe-2026-09-16.md`); +if your kiro-cli is newer, re-run the receipt's two probes before trusting +the grant. The same correction reached the Kiro `study-mentor` on +2026-09-16: its twelve MCP grants had used the `mcp_` spelling and were +inert, so after `studyloop install agents` an existing mentor install starts +seeing and using the six `studyloop` tools and the six `session-db` tools its +file always named. OpenCode, Codex and Grok Build register the server globally (`studyloop install agents` writes it into each harness's own MCP configuration), so their architects reach the tools without a per-agent grant; pi has no MCP client and takes the CLI fallback the persona describes diff --git a/docs/architecture/plan-integration/receipts/kiro-agent-tools-probe-2026-09-16.md b/docs/architecture/plan-integration/receipts/kiro-agent-tools-probe-2026-09-16.md index d9dba6de5..fc04572b0 100644 --- a/docs/architecture/plan-integration/receipts/kiro-agent-tools-probe-2026-09-16.md +++ b/docs/architecture/plan-integration/receipts/kiro-agent-tools-probe-2026-09-16.md @@ -1,5 +1,12 @@ # Kiro agent-config probe — how `tools` / `allowedTools` govern MCP tools · 2026-09-16 +> **Version-pinned evidence.** Every rule below was observed on kiro-cli **2.21.4** and re-observed on +> **2.22.0** (§ Re-run). If the installed `kiro-cli --version` is newer than 2.22.0, re-run probes A and B +> below before trusting the `@server/tool` spelling the architect and mentor grants depend on, and append +> the result as a dated re-run section. Not probed here: `@session-db` in `tools` with nothing from that +> server in `allowedTools` — design §1's "visible, prompts" reading of that shape is an expectation, not a +> measurement (council review 6). + **Why this exists.** Item 1 of the follow-on programme (owner decision D-A) grants the harness-launched `study-plan-architect` the `studyloop` MCP server "mirroring `agents/kiro/study-mentor.json`". Before pinning that shape in a test, the coordinator checked what the installed Kiro CLI actually honours, because the diff --git a/packages/studyloop/tests/test_plan_architect_persona.py b/packages/studyloop/tests/test_plan_architect_persona.py index 6bedb8ca9..ca6e106ba 100644 --- a/packages/studyloop/tests/test_plan_architect_persona.py +++ b/packages/studyloop/tests/test_plan_architect_persona.py @@ -406,6 +406,20 @@ def test_install_docs_disclose_architect_fallback_limits() -> None: assert "open item" not in lowered and "stay cli-limited" not in lowered, ( "the decision has been taken; the doc must not describe it as open" ) + # Council review 6 (GPT F5 / Grok 🔵 c, d, a): the Claude allow-list names + # tools; the doc must say where the SERVER is registered for Claude, and + # that path must be the installer's own — not a remembered one. Existing + # mentor installs gain live tools from the spelling fix; the grant spelling + # is pinned to a kiro-cli version and the doc must say when to re-probe. + from studyloop import installers + + assert "claude" in installers._MCP_HARNESSES + claude_mcp = installers._mcp_config_path("claude") + assert f"`~/{claude_mcp.relative_to(installers._HOME)}`" in section, ( + "the doc must name the file the installer registers the studyloop server in for Claude" + ) + assert "study-mentor" in section and "inert" in lowered, "mentor grant activation" + assert "2.22.0" in section and "re-run" in lowered, "the version-pinned probe" def test_fallback_table_does_not_point_at_web_ui_controls_that_do_not_exist() -> None: From 6d01d919656ff70d85fd9cdbd77f0ea06aafe6c6 Mon Sep 17 00:00:00 2001 From: NetDevAutomate Date: Fri, 18 Sep 2026 11:30:34 +0100 Subject: [PATCH 12/23] fix(web): the Today card keeps each closing review's evidence with its plan; the label says review, not complete (council review 6, F6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card rendered every completion sentence, then every evidence line flattened beneath them, so with two finished plans a line lost the plan it belonged to — the contextual review D-G asks for, weakened at the surface most learners read. And both the card and CLI now labelled the note 'Plan complete' over a plan whose status is still active until the learner agrees with the architect. GPT-Astra F6 🟡; reproduced from the markup. today-panel.js gains completionReviews(): one block per action — sentence and its own evidence lines — keyed by plan_id in the engine's order; the flat completionNotes()/completionEvidence() helpers now derive from it. index.html renders one .today-plan-review block per plan (data-plan-id) with the lines nested inside, labelled 'Closing review'; cli/_now.py prints the same label. A JS test pins the grouping (an extend review, a partial review carrying its 'Not read:' line, and a pre-D-G entry) and a markup test pins the keyed block, the nesting and the absence of both a flat evidence loop and the word 'Plan complete'. No test had pinned either old label. JS 139/139 (+2), CLI now/guidance/seam 67/67, e2e Today/plan subset 29 passed; design §4 records the correction. --- .../plan-integration-followons/design.md | 4 +- packages/studyloop/src/studyloop/cli/_now.py | 4 +- .../src/studyloop/web/static/index.html | 17 +++++-- .../web/static/js/components/today-panel.js | 29 +++++++---- .../tests/js/today-panel-plan.test.js | 50 +++++++++++++++++++ 5 files changed, 88 insertions(+), 16 deletions(-) diff --git a/openspec/changes/plan-integration-followons/design.md b/openspec/changes/plan-integration-followons/design.md index 08793e22c..2fdd707e6 100644 --- a/openspec/changes/plan-integration-followons/design.md +++ b/openspec/changes/plan-integration-followons/design.md @@ -185,7 +185,9 @@ among what MCP revises and the row names every schema property. returns `None` for the whole review on failure — so "unassessed" is represented once, at the action, not twice. - **Two more GREEN-time decisions (`82293293`):** (1) the Today card gained `completionEvidence()` and renders - the review's evidence lines under the "Plan complete" note, matching CLI `now`'s dim lines — the design said + the review's evidence lines under the "Plan complete" note, matching CLI `now`'s dim lines (council review 6 F6 + corrected both: the card renders one `completionReviews()` block per plan — sentence, then *its* lines, keyed by + `plan_id` — and the label on the card and in CLI `now` is "Closing review", since the plan is still `active`) — the design said the card prints "the proposal and the counts", which the sentence carries, but the surface most learners read should also show what the proposal rests on; a pre-D-G entry without `evidence`, or a failed assessment, contributes nothing. (2) `docs/study-plans.md`'s "Deliberately not automatic" list is the pinned six-item diff --git a/packages/studyloop/src/studyloop/cli/_now.py b/packages/studyloop/src/studyloop/cli/_now.py index 938667ab1..1d9179060 100644 --- a/packages/studyloop/src/studyloop/cli/_now.py +++ b/packages/studyloop/src/studyloop/cli/_now.py @@ -70,7 +70,9 @@ def _render_plan(plan) -> None: f"{deferred.energy_capability}/10. Plan-related review and repair stay available." ) for completion in getattr(plan, "completion_actions", ()): - console.print(f"[green]Plan complete:[/green] {escape(completion.action)}") + # "Closing review", not "Plan complete": the status is still active until + # the learner agrees with the architect (council review 6, F6). + console.print(f"[green]Closing review:[/green] {escape(completion.action)}") # The review's evidence, one dim line per counted item (D-G); the # sentence above already carries the proposal and the counts. for line in getattr(completion, "evidence", ()): diff --git a/packages/studyloop/src/studyloop/web/static/index.html b/packages/studyloop/src/studyloop/web/static/index.html index 2a832a8a6..c23e0982e 100644 --- a/packages/studyloop/src/studyloop/web/static/index.html +++ b/packages/studyloop/src/studyloop/web/static/index.html @@ -1110,11 +1110,18 @@

- -