From 7f2769c529b10a6d174447555e02aac29666e0b7 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:07:25 +0800 Subject: [PATCH] docs: address latest simplification review --- docs/rfc/README.md | 5 +- .../2026-06-20-classify-packages-by-aspect.md | 46 +++++++++++++++++++ .../2026-06-20-discover-package-inventory.md | 2 +- ...0-remove-redundant-snapshot-log-goldens.md | 20 ++++---- .../2026-06-20-classify-support-packages.md | 2 +- ...2026-06-20-drop-durable-step-boundaries.md | 6 +-- .../2026-06-20-retire-mid-turn-steering.md | 2 +- .../2026-06-20-truncate-interrupted-turns.md | 2 +- 8 files changed, 68 insertions(+), 17 deletions(-) create mode 100644 docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md rename docs/rfc/{proposed => rejected}/2026-06-20-drop-durable-step-boundaries.md (67%) diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 84677bda8a..6e2fbb9a7e 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -37,11 +37,11 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Drop unconsumed assembled LLM convenience surfaces](proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md) | 2026-06-20 | | [Drop the unconsumed `llm/adapter-change` event](proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) | 2026-06-20 | | [Prune dead methods from the persistence and bash seams](proposed/2026-06-20-prune-dead-seam-methods.md) | 2026-06-20 | -| [Drop durable step boundary events](proposed/2026-06-20-drop-durable-step-boundaries.md) | 2026-06-20 | | [Fold trace-only session facts into load-bearing events](proposed/2026-06-20-collapse-trace-only-session-events.md) | 2026-06-20 | | [Extract a generic long-running tool runtime](proposed/2026-06-20-generic-long-running-tool-runtime.md) | 2026-06-20 | | [Make the shared example base providerless](proposed/2026-06-20-providerless-example-base.md) | 2026-06-20 | -| [Use the recorded session fixture as the snapshot log golden](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | +| [Use `session.jsonl` as the only snapshot session-log artifact](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | +| [Classify packages by aspect metadata](proposed/2026-06-20-classify-packages-by-aspect.md) | 2026-06-20 | | [Discover package inventories instead of maintaining static lists](proposed/2026-06-20-discover-package-inventory.md) | 2026-06-20 | ## Implemented @@ -84,6 +84,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Drop ACP session/load until resume has a product shape](rejected/2026-06-20-drop-acp-session-load.md) | 2026-06-20 | | [Drop ACP terminal `_meta` rendering](rejected/2026-06-20-drop-acp-terminal-meta.md) | 2026-06-20 | | [Drop bash full-output spill files](rejected/2026-06-20-drop-bash-output-spill-files.md) | 2026-06-20 | +| [Drop durable step boundary events](rejected/2026-06-20-drop-durable-step-boundaries.md) | 2026-06-20 | | [Drop unused session lineage metadata](rejected/2026-06-20-drop-unused-session-lineage.md) | 2026-06-20 | | [Fold the persistence interface into dsh-session](rejected/2026-06-20-fold-session-persistence-interface.md) | 2026-06-20 | | [Collapse tool-owned UI presentation](rejected/2026-06-20-generic-tool-rendering.md) | 2026-06-20 | diff --git a/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md b/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md new file mode 100644 index 0000000000..e9723a5660 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md @@ -0,0 +1,46 @@ +# RFC: Classify packages by aspect metadata + +Status: proposed + +## Problem + +The harness package tree is flat, and every package manifest is currently `private: true`. That is fine as a pre-release safety default, but it means neither paths nor npm publish flags tell scripts what role a package plays. [publint-all](../../../scripts/publint-all.ts) needs to know which packages are release-shaped, docs need to describe which packages are core product surface, and future cleanup work needs a way to distinguish support utilities from load-bearing product modules. + +A single hierarchy such as product, integration, support, or testing is too coarse. Packages naturally carry overlapping facts: an LLM adapter is provider-facing and publish-shaped; `tool-bash` is a tool consumer and bash-related; `llm-replay` is an LLM adapter shape and test/snapshot support; ACP is an editor bridge and current product surface. Forcing each package into one bucket would either hide useful facts or recreate static exception lists under different names. + +## Proposal + +Add explicit, repo-owned package aspect metadata to each `packages/*/package.json`, using a manifest-local key such as `dsh.aspects` unless the implementing change finds an already-established repo metadata key. The metadata is a controlled vocabulary, not free-form prose. + +For example: + +```json +{ + "dsh": { + "aspects": ["core", "llm", "publishable"] + } +} +``` + +The initial vocabulary should stay small and useful to scripts. Expected facets include `core`, `implementation`, `consumer`, `llm`, `bash`, `fs`, `persistence`, `agent`, `acp`, `ui`, `example-support`, `test-support`, `replay`, and `publishable`. A package may declare multiple facets; no script should assume exactly one role. + +`publishable` is a repo policy facet, not a mirror of npm's `private` flag. While the harness is unreleased, packages can remain `private: true` and still declare `publishable` so publish-shape gates know which manifests to check. When release policy changes, the aspect continues to describe intent while the npm flag controls whether publication is allowed. + +Scripts should consume the metadata directly. `publint-all` filters on `publishable`, module graph or package inventory docs can group by domain facets, and the adding-a-package cookbook asks authors to choose aspects when creating a new package. Unknown facets should fail loudly so typoed metadata does not silently fork the taxonomy. + +## Acceptance criteria + +- Every `packages/*` manifest declares package aspects from a documented controlled vocabulary. +- The vocabulary explains each facet's meaning and when a new facet is appropriate. +- `publint-all` derives its package list from `publishable` metadata instead of a hard-coded array. +- Package inventory docs and module-graph grouping can read aspects without inferring intent from package names or folder paths. +- Adding a package requires choosing aspects, and CI fails if a package is missing aspect metadata or uses an unknown facet. +- No package path moves are required just to express classification. + +## What we give up + +Aspect metadata is less visually obvious than folders, and a package can be over-tagged if reviewers are careless. The counterweight is that metadata preserves the current package import shape while making policy facts explicit and machine-checkable. If a future package truly needs a new physical boundary, that move can still happen for architectural reasons rather than as a classification workaround. + +## Related + +This supersedes the rejected [product/integration/support package taxonomy](../rejected/2026-06-20-classify-support-packages.md) and supplies the package source of truth expected by [discover package inventories](2026-06-20-discover-package-inventory.md). diff --git a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md index c75f93fd99..c99412c14b 100644 --- a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md @@ -12,7 +12,7 @@ Static lists are appropriate when they encode policy; they are needless friction Make package/gate inventories discoverable. Publishability should come from explicit package aspect metadata, not from a static array in a script or the npm `private` flag. Module graph generation should read package manifests. `doc-sync` should be the one command that defines and prints its sub-gates, with docs linking to that command rather than restating a second list. -The metadata should be aspect-oriented rather than a single support/product bucket: a package may be core, bash-related, filesystem-related, persistence-related, provider-facing, example-facing, testing-only, and/or publishable. Discovery needs enough explicit facts to drive gates without baking a fragile hierarchy into every script. +The metadata should come from [classifying packages by aspect](2026-06-20-classify-packages-by-aspect.md) rather than a single support/product bucket: a package may be core, bash-related, filesystem-related, persistence-related, provider-facing, example-facing, testing-only, and/or publishable. Discovery needs enough explicit facts to drive gates without baking a fragile hierarchy into every script. ## Acceptance criteria diff --git a/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md b/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md index cf853ce341..cb40dbbf9f 100644 --- a/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md +++ b/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md @@ -1,27 +1,31 @@ -# RFC: Use the recorded session fixture as the snapshot log golden +# RFC: Use `session.jsonl` as the only snapshot session-log artifact Status: proposed ## Problem -Recorded ACP snapshot scenarios ship both `session.jsonl` and `session.golden.jsonl`. For normal recorded scenarios, `session.jsonl` is the replay fixture harvested from a real run, and the replay test normalizes the newly persisted log and compares it to `session.golden.jsonl`. In the current fixtures, the normalized recorded log and normalized golden are identical for the ordinary recorded scenarios. +Model-driving ACP snapshot scenarios ship both `session.jsonl` and `session.golden.jsonl`. For normal recorded scenarios, `session.jsonl` is the replay fixture harvested from a real run, and the replay test normalizes the newly persisted log and compares it to `session.golden.jsonl`. In the current fixtures, the normalized recorded log and normalized golden are identical for the ordinary recorded scenarios. -The duplicate file can help review by showing "expected persisted log" separately from "model replay input", but for recorded scenarios those are intentionally the same artifact. Keeping both means a re-record churns two files with the same semantic content, when one committed session log can serve as both replay input and expected persisted output. +Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario. ## Proposal -For recorded scenarios, keep one session-log artifact: `session.jsonl`. The snapshot test compares the replay run's normalized persisted log directly against normalized `session.jsonl`. Keep explicit `session.golden.jsonl` only for authored scenarios where `replay.override.json` drives behavior that is not derivable from the fixture, or where the expected persisted log intentionally differs from the replay script. +Remove the `session.golden.jsonl` concept entirely. Every scenario has at most one committed session-log artifact, `session.jsonl`: + +- For recorded scenarios, `session.jsonl` remains the raw harvested log. Replay still derives model chunks from it, and the snapshot test compares the replay run's normalized persisted log against normalized `session.jsonl`. +- For authored override scenarios, `replay.override.json` drives model behavior and `session.jsonl` holds the expected produced session log. The replay adapter ignores the fixture for model chunks when the override exists, so the same file can be the expected log without affecting replay behavior. +- For no-model scenarios, `session.jsonl` can stay as the minimal fixture needed to boot `llm-replay`; no session-log comparison is needed unless the scenario creates a persisted session. Stdout goldens remain unchanged; they are the editor-facing projection and are not redundant with the session fixture. ## Acceptance criteria -- Recorded scenarios commit `session.jsonl` as the single session-log fixture/golden and stop committing `session.golden.jsonl`. -- The snapshot test derives the expected session log from `session.jsonl` for `recorded: true` scenarios. -- Authored sidecar scenarios keep explicit session goldens when needed. +- `session.golden.jsonl` disappears from the snapshot harness, fixtures, orphan guards, and docs. +- The snapshot test derives the expected session log from `session.jsonl` for every model scenario. +- Authored sidecar scenarios commit their expected produced log in `session.jsonl`; `replay.override.json` remains the model-behavior override. - Orphan-fixture guards understand which files are required by scenario kind. - The [ACP snapshot tests RFC](../implemented/2026-06-19-acp-snapshot-tests.md) is updated to describe the reduced fixture set. ## What we give up -Reviewers lose one redundant artifact that made the expected persisted log visually separate from the replay fixture. The stdout golden still protects the editor transcript, and comparing replay output to the recorded fixture preserves the loop/persistence regression check without duplicating files. +Reviewers lose one artifact name that made the expected persisted log visually separate from the replay fixture. The stdout golden still protects the editor transcript, and comparing replay output to `session.jsonl` preserves the loop/persistence regression check without duplicating files. diff --git a/docs/rfc/rejected/2026-06-20-classify-support-packages.md b/docs/rfc/rejected/2026-06-20-classify-support-packages.md index 3634974a5b..254ddbdc7c 100644 --- a/docs/rfc/rejected/2026-06-20-classify-support-packages.md +++ b/docs/rfc/rejected/2026-06-20-classify-support-packages.md @@ -12,7 +12,7 @@ This is not just cosmetic. A package's location currently says little about whet Introduce an explicit package classification and move packages accordingly, for example `packages/core/`, `packages/integrations/`, `packages/tools/`, `packages/testing/`, and `packages/examples/`, or an equivalent structure decided in the implementing PR. The important part is that example/test support packages are not indistinguishable from product core. -The rejected part is the one-dimensional taxonomy. The useful follow-up is explicit package aspect metadata that scripts can consume without pretending a package has only one role. +The rejected part is the one-dimensional taxonomy. The useful follow-up is [explicit package aspect metadata](../proposed/2026-06-20-classify-packages-by-aspect.md) that scripts can consume without pretending a package has only one role. This proposal does not delete `llm-replay` or `ui-stdio` by itself. It makes their status honest: either they graduate into product packages with documented consumers, or they live under a support/testing/example classification where release and compatibility expectations are lower. diff --git a/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md b/docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md similarity index 67% rename from docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md rename to docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md index ad424cb06e..60b2af391a 100644 --- a/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md +++ b/docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md @@ -1,12 +1,12 @@ # RFC: Drop durable step boundary events -Status: proposed +Status: rejected — `step/end` is the durable indication that a model step finished, and keeping the symmetric `step/start` / `step/end` pair makes crash repair, invariants, and transcript inspection clearer than inferring completion from adjacent step-scoped events. ## Problem The session log stores `step/start` and `step/end` events even though every step-scoped event already carries `{ turn, step }`: assistant chunks, assistant messages, tool calls, tool results, usage, and errors. `deriveMessages()` ignores step boundaries, ACP ignores them for UI, and the main consumers are invariants, tests, snapshot goldens, and crash repair. -The boundary events make the log more ceremonial than informative. The loop tracks open steps solely to close them, repair synthesizes `step/end` when a crash leaves a step open, invariants track a second nesting stack inside the turn, and snapshots carry lines that do not affect replayed message history. A model request that crashes before producing any step-scoped event is the only information represented by a bare `step/start`, and that case has no useful resumable content. +The rejected argument was that boundary events make the log more ceremonial than informative. In practice, `step/end` is concrete information: a reader can tell whether a model request finished, crashed, or is being repaired without deriving that state from the next event. A bare `step/start` is likewise useful for a model request that began but produced no chunks before failing. ## Proposal @@ -25,4 +25,4 @@ The invariants plugin should enforce that step-scoped events have valid positive ## What we give up -The log no longer records "a model request started but produced no event before the process died" as a durable fact. That is acceptable: there is no assistant content, tool call, usage, or error to replay from that empty request. A live UI can still show an in-progress step from a transient event if it needs one; the durable log should not store an empty bracket. +The log no longer records "a model request started but produced no event before the process died" as a durable fact, and no longer has an explicit "this step completed" marker. That loss is not acceptable while the session log is the durable replay and audit surface. diff --git a/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md b/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md index cbfa238370..97c7d5dc61 100644 --- a/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md +++ b/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md @@ -30,4 +30,4 @@ A user cannot add same-turn steering content while a model is between tool steps ## Related -This pairs naturally with [dropping durable step boundaries](../proposed/2026-06-20-drop-durable-step-boundaries.md), because removing same-turn steering and `agent/turn-continuation` leaves tool calls as the only reason a turn contains multiple model steps. +This pairs naturally with [dropping durable step boundaries](2026-06-20-drop-durable-step-boundaries.md), because removing same-turn steering and `agent/turn-continuation` leaves tool calls as the only reason a turn contains multiple model steps. diff --git a/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md b/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md index 410237daa4..771388ede9 100644 --- a/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md +++ b/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md @@ -29,4 +29,4 @@ A crash can lose real work from the final turn: assistant text, tool calls, and ## Related -This is a direct simplification of [session persistence](../implemented/2026-06-14-session-persistence.md) and [turn enclosure](../implemented/2026-06-15-turn-enclosure-invariant.md). It also removes much of the motivation for durable step boundary events, making [drop durable step boundary events](../proposed/2026-06-20-drop-durable-step-boundaries.md) smaller. +This is a direct simplification of [session persistence](../implemented/2026-06-14-session-persistence.md) and [turn enclosure](../implemented/2026-06-15-turn-enclosure-invariant.md). It also removes much of the motivation for durable step boundary events, making [drop durable step boundary events](2026-06-20-drop-durable-step-boundaries.md) smaller.