docs: address latest simplification review
This commit is contained in:
46
docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md
Normal file
46
docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md
Normal file
@@ -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).
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -1,28 +0,0 @@
|
||||
# RFC: Drop durable step boundary events
|
||||
|
||||
Status: proposed
|
||||
|
||||
## 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.
|
||||
|
||||
## Proposal
|
||||
|
||||
Make the turn the only durable boundary. Remove `step/start` and `step/end` from `SessionEventMap`; keep the numeric `step` field on events that need grouping. The loop increments the step counter and records step-scoped events with that number, but it no longer appends open/close boundary events. Consumers infer step groups from contiguous events sharing `(turn, step)`.
|
||||
|
||||
The invariants plugin should enforce that step-scoped events have valid positive step numbers within an open turn, not that separate boundary records surround them. Crash repair should not synthesize `step/end`; if an interrupted turn is preserved, the repair path can still close the turn without inventing step boundary records.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `SessionEventMap` no longer includes `step/start` or `step/end`.
|
||||
- The loop has no `closeStep()` finalization path.
|
||||
- ACP snapshots and persistence contract fixtures stop expecting step-boundary lines.
|
||||
- `deriveMessages()` and replay derive the same message history from step-scoped events.
|
||||
- The [event taxonomy docs](../../architecture.md) describe turns as the durable boundary and steps as a field on step-scoped records.
|
||||
- The session format version and recorded fixtures are refreshed; non-current stored logs are rejected per the pre-release format policy.
|
||||
|
||||
## 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.
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user