526 lines
25 KiB
Markdown
526 lines
25 KiB
Markdown
# Contract Simplification Audit
|
|
|
|
## Status
|
|
|
|
Complete and approved as a statement of intended direction. This document
|
|
records product and architecture decisions for future work; it does not describe
|
|
implemented behavior and is not an implementation plan.
|
|
|
|
This audit applies the same contract-challenge premise used by the
|
|
[File Operations Simplification roadmap](fileops.md) to other maintainability
|
|
findings in [`audit.md`](audit.md): current behavior is not automatically a
|
|
durable product requirement merely because production code, tests, or
|
|
documentation currently preserve it.
|
|
|
|
## Context
|
|
|
|
Narratio is a trusted, single-session workflow orchestrator for intentionally
|
|
shareable D&D material. Its most important operational outcomes are correct
|
|
pipeline progress, economical reuse of expensive successful work, coherent
|
|
remote publication, understandable recovery, safe cleanup scope, and protection
|
|
of API keys. It does not need compliance-grade invocation history, indefinite
|
|
compatibility with already-deprecated formats, or distributed coordination
|
|
machinery disproportionate to its actual use.
|
|
|
|
The maintainability audit found several areas where implementation mechanisms
|
|
had become apparent contracts. This follow-up review explicitly permits those
|
|
contracts to change when a simpler policy preserves the user outcome that
|
|
matters.
|
|
|
|
## Governing Decisions
|
|
|
|
- Remove deprecated legacy and compatibility behavior completely rather than
|
|
continuing to isolate and maintain it.
|
|
- Keep the session manifest as the sole durable local progress authority.
|
|
- Preserve successful analyze artifacts individually and checkpoint them as
|
|
they complete.
|
|
- Express local cleanup as an ordinary final pipeline stage rather than a
|
|
publish side effect or special manifest obligation.
|
|
- Represent each remote artifact lock as one independently managed remote
|
|
object and carry locked artifacts forward into later current snapshots.
|
|
- Preserve configuration semantics and useful diagnostics without treating
|
|
exact validation order, prose, or formatting as compatibility contracts.
|
|
- Release this work as a minor version. The removed compatibility surfaces were
|
|
already deprecated and explicitly subject to removal; their deletion does not
|
|
require a major-version release under the project decision recorded here.
|
|
|
|
## Finding 1 — Deprecated Compatibility Has Become Cross-Cutting Product Code
|
|
|
|
### Underlying issue
|
|
|
|
Narratio retains multiple compatibility paths for state and configuration that
|
|
predate current canonical contracts. These paths include legacy remote current
|
|
state, candidate probing for previous-session artifacts, unversioned party and
|
|
separate players inputs, aggregate-only analyze state, older extraction
|
|
evidence, and manifests without current semantic evidence.
|
|
|
|
Although several paths are labeled temporary or migration-only, their presence
|
|
still requires alternate parsing and selection algorithms, ambiguity handling,
|
|
branches in current workflows, compatibility-specific errors, fixtures, tests,
|
|
and documentation. Isolating these paths makes later removal easier, but does
|
|
not remove their ongoing cost. Continued support also makes it harder to reason
|
|
about which representation is authoritative.
|
|
|
|
### Approved changes
|
|
|
|
Purge deprecated compatibility behavior across production code, tests,
|
|
configuration, documentation, examples, and schemas. The purge includes:
|
|
|
|
- the legacy remote `current/manifest.json` and `current/run_id.txt` reader;
|
|
- legacy previous-session artifact path candidates and existence probing;
|
|
- unversioned party documents and separate `players_file` behavior;
|
|
- legacy session-level party and players overrides that exist only for that
|
|
input model;
|
|
- aggregate-only legacy analyze availability and fallback to incidental files;
|
|
- compatibility for extraction records that lack the current contract and
|
|
provenance evidence;
|
|
- compatibility branches for manifests or artifact records that predate the
|
|
current supported schema; and
|
|
- associated legacy constants, types, helpers, errors, tests, fixtures,
|
|
documentation, and examples.
|
|
|
|
Current state must have an explicit supported version or enough required
|
|
authority markers to be recognized unambiguously. Unsupported old local or
|
|
remote state must fail early with a clear migration/version error. Narratio must
|
|
not silently normalize, reinterpret, probe around, or partially accept it.
|
|
|
|
Migration guidance may explain how to regenerate, republish, or otherwise
|
|
replace old state, but migration readers must not remain in the steady-state
|
|
runtime merely to preserve automatic compatibility.
|
|
|
|
### Rationale
|
|
|
|
Compatibility was already deprecated and bounded for removal. Retaining it
|
|
indefinitely converts a temporary cost into permanent architecture. One
|
|
canonical representation is easier to validate, test, document, restore, and
|
|
extend, and a clear rejection is safer than heuristic interpretation of old
|
|
state.
|
|
|
|
### Final state
|
|
|
|
Every supported configuration, party document, local manifest, remote current
|
|
snapshot, extraction record, analyze record, and previous-session artifact is
|
|
resolved through one canonical contract. Deprecated formats are absent from
|
|
production code and tests and receive a clear unsupported-version or migration
|
|
error rather than fallback behavior.
|
|
|
|
## Finding 2 — Invocation Diagnostics Participate in Pipeline Correctness
|
|
|
|
### Underlying issue
|
|
|
|
Narratio currently maintains both an authoritative session manifest and a
|
|
structured run manifest for each invocation. The runner persists overlapping
|
|
stage actions, statuses, outputs, semantic evidence, failures, and terminal
|
|
state to both documents. This creates dual-save ordering, joined persistence
|
|
errors, separate session and invocation projections, terminal-failure recovery
|
|
machinery, and tests for keeping a diagnostic record synchronized with the
|
|
actual progress ledger.
|
|
|
|
Historical per-invocation records are not operationally important. They are at
|
|
most useful diagnostics and do not justify being a second participant in
|
|
pipeline transactions. Their placement inside a run directory that cleanup may
|
|
delete further demonstrates that they are not durable progress authority.
|
|
|
|
### Approved changes
|
|
|
|
Eliminate the structured run manifest and its persistence lifecycle.
|
|
|
|
Retain:
|
|
|
|
- the run ID used for invocation identity, run-local paths, adapter provenance,
|
|
and immutable remote publication;
|
|
- the session manifest as the sole durable local record of current pipeline and
|
|
artifact state;
|
|
- ordinary bounded logs, generated configuration, receipts, and other
|
|
diagnostics where already useful; and
|
|
- the in-memory command result or summary reported to the caller.
|
|
|
|
Remove:
|
|
|
|
- run-manifest types, storage methods, path helpers, schema and validation;
|
|
- before-stage, after-stage, skip, failure, and terminal run-manifest saves;
|
|
- invocation-only analyze projections and output projections;
|
|
- publish handling that archives or classifies a run manifest;
|
|
- terminal persistence code whose only purpose is to coordinate session and
|
|
run records; and
|
|
- run-manifest documentation, compatibility handling, fixtures, and tests.
|
|
|
|
No replacement historical record is required. A lightweight diagnostic summary
|
|
may be reconsidered later only if it can remain independent of pipeline
|
|
correctness and cleanup.
|
|
|
|
### Rationale
|
|
|
|
One durable authority eliminates synchronization and split-brain questions.
|
|
The session manifest already records the state needed for resume, status,
|
|
restore, artifact resolution, and publication. Logs and the immediate command
|
|
summary provide proportionate invocation diagnostics without making diagnostic
|
|
write failures fail otherwise valid pipeline work.
|
|
|
|
### Final state
|
|
|
|
The session manifest alone determines durable local pipeline progress. A run ID
|
|
still identifies an invocation and its files, but no second manifest mirrors
|
|
session transitions. Pipeline success or failure cannot be changed by an error
|
|
writing historical invocation metadata.
|
|
|
|
## Finding 3 — Analyze Recovery Is Obscured by Dual Projection Machinery
|
|
|
|
### Underlying issue
|
|
|
|
Narratio correctly models configured analyze artifacts individually, but the
|
|
implementation combines that valuable behavior with separate session and
|
|
invocation collections, partial-result projection validation, rollback of
|
|
in-memory authority after persistence failure, and special runner handling for
|
|
a stage that returns both a result and an error.
|
|
|
|
The desired user behavior is simpler than the machinery. Expensive successful
|
|
artifacts should remain reusable; a failed artifact and work that depends on it
|
|
should be retried. Invocation-specific audit projections are unnecessary once
|
|
run manifests are removed.
|
|
|
|
### Approved behavior
|
|
|
|
Assume an ordered artifact plan contains A, B, C, and D. If A and B succeed, C
|
|
fails, and D is not attempted:
|
|
|
|
- A and B remain current, provided their output and semantic evidence still
|
|
validate;
|
|
- C is recorded as failed and has no current output authority;
|
|
- D remains missing if it has never succeeded, or becomes stale if an older
|
|
current D depends directly or transitively on C;
|
|
- the aggregate analyze stage is failed and execution stops;
|
|
- an ordinary later run or resume retains A and B, retries C, and proceeds to D
|
|
after C succeeds; and
|
|
- a forced analyze invocation reruns the effective selection regardless of
|
|
current records.
|
|
|
|
Without `--artifacts`, force applies to the full configured analyze artifact
|
|
set. With `--artifacts`, force applies to the selected artifacts and the
|
|
dependency closure required by the established artifact-selection contract;
|
|
unrelated valid artifacts remain outside that invocation.
|
|
|
|
### Approved changes
|
|
|
|
Use the session manifest's analyze artifact collection as the only durable
|
|
artifact authority. Remove the invocation collection and all logic that exists
|
|
only to construct, validate, apply, or roll back an invocation-specific
|
|
projection.
|
|
|
|
Execute the effective artifact set in deterministic dependency order. After
|
|
each artifact succeeds and its canonical output has been validated and
|
|
materialized:
|
|
|
|
1. construct or update its current session artifact record;
|
|
2. stale any current transitive dependents when the new result invalidates
|
|
their dependency evidence;
|
|
3. atomically save the session manifest; and
|
|
4. only then proceed to the next artifact.
|
|
|
|
If the checkpoint save fails, the new file alone does not establish authority;
|
|
the invocation fails and the artifact is eligible to run again. This safe
|
|
recomputation is preferable to a rollback transaction around incidental output
|
|
bytes.
|
|
|
|
When an artifact fails, record its failed state, stale current transitive
|
|
dependents, mark the aggregate stage failed, and atomically save those changes
|
|
to the session manifest before returning the artifact error. A later run
|
|
reconciles each selected artifact from the persisted collection instead of
|
|
rerunning the entire stage automatically.
|
|
|
|
Retain:
|
|
|
|
- per-artifact fingerprints and validated output evidence;
|
|
- deterministic dependency ordering;
|
|
- selected-artifact and dependency-closure semantics;
|
|
- invalidation when configuration, inputs, dependencies, or output identity
|
|
changes; and
|
|
- the rule that incidental canonical files never create current authority.
|
|
|
|
Simplify failure and persistence tests around these observable checkpoints
|
|
rather than the current projection and rollback choreography.
|
|
|
|
### Rationale
|
|
|
|
Checkpointing after each successful artifact matches the economic reality that
|
|
LLM work is expensive while one manifest write is cheap. It preserves A and B
|
|
after an ordinary error, cancellation, or process interruption. The manifest
|
|
remains the one authority, and a failed checkpoint safely causes recomputation
|
|
rather than ambiguous reuse.
|
|
|
|
Dependency-aware reuse is worth retaining because character families,
|
|
artifact selection, and ongoing prompt development make whole-stage reruns
|
|
unnecessarily expensive. Invocation-only state is not.
|
|
|
|
### Final state
|
|
|
|
Analyze behaves as a resumable deterministic artifact queue backed by one
|
|
session collection. Successful valid artifacts survive later failures, the
|
|
first failed or invalid artifact is retried, downstream work resumes when its
|
|
prerequisites become current, and `--force` deliberately replaces normal reuse
|
|
within its effective selection.
|
|
|
|
## Finding 4 — Cleanup Is a Hidden Publish Transaction
|
|
|
|
### Underlying issue
|
|
|
|
Remote publication and local deletion are independent operator outcomes, but
|
|
Narratio currently couples them. After publish commits remotely, the runner
|
|
creates a special durable cleanup obligation in the session manifest, deletes
|
|
eligible local targets one by one, persists completion evidence after each,
|
|
and reports the invocation as incomplete if cleanup fails. An operator can
|
|
therefore observe an apparent publish failure even though the remote commit is
|
|
already authoritative.
|
|
|
|
The special obligation duplicates behavior already provided by pipeline stage
|
|
status and resume.
|
|
|
|
### Approved changes
|
|
|
|
Add `cleanup` as the final stage in the canonical pipeline, after every stage
|
|
that may need run-local material or diagnostics. Remove post-publish cleanup
|
|
from the runner and remove the special `post_publish_cleanup` manifest model.
|
|
|
|
The cleanup stage must:
|
|
|
|
- use existing cleanup configuration to determine whether spool audio, run
|
|
workspace data, both, or neither should be removed;
|
|
- require evidence of a successful current remote publish before deleting data
|
|
whose retention policy is conditioned on publication;
|
|
- bind cleanup targets to the applicable successfully published run rather than
|
|
infer arbitrary paths from current configuration;
|
|
- self-skip clearly when no cleanup is configured;
|
|
- use the root-scoped destructive safeguards defined by
|
|
[`fileops.md`](fileops.md);
|
|
- treat an already-absent eligible target as success so retry is idempotent;
|
|
- record ordinary stage success or failure in the session manifest; and
|
|
- produce no canonical artifact outputs.
|
|
|
|
A cleanup failure must not alter the succeeded publish record or remote current
|
|
state. A later ordinary run skips still-valid upstream work and retries cleanup.
|
|
If an earlier stage is forced or otherwise changes, the fixed invalidation
|
|
relation must make the prior cleanup result stale where appropriate.
|
|
|
|
Selecting a range that ends before cleanup must not delete local data. A full
|
|
default run includes cleanup because it is the canonical final stage.
|
|
|
|
### Rationale
|
|
|
|
Stage boundaries should match outcomes an operator can understand. Publication
|
|
has succeeded when the remote commit becomes current; local deletion has
|
|
succeeded when the selected local targets are absent. Modeling them separately
|
|
gives each its own status, errors, retry, selection, and documentation without a
|
|
custom transaction layered after the stage loop.
|
|
|
|
The removal of run manifests also eliminates the present problem in which
|
|
cleanup deletes the directory containing an invocation record that the runner
|
|
otherwise attempts to finalize.
|
|
|
|
### Final state
|
|
|
|
`publish` owns only coherent remote publication. `cleanup` is the final,
|
|
idempotent local-maintenance stage. A failure is reported as `cleanup` failure,
|
|
publish remains visibly successful, and normal resume retries cleanup without
|
|
republishing unchanged artifacts.
|
|
|
|
## Finding 5 — Remote Locks Use a Shared Mutable Control Plane
|
|
|
|
### Underlying issue
|
|
|
|
The current lock system stores remote locks in one mutable `locks.yml`, merges
|
|
them with static pipeline locks, defines precedence, validates the combined
|
|
document, uses generation-bound conditional replacement and bounded retries to
|
|
avoid concurrent lost updates, injects the effective set into runtime
|
|
configuration, and reloads it before current-pointer selection.
|
|
|
|
This complexity follows from choosing one shared mutable document and two lock
|
|
authorities. The product requirement is narrower: an already canonical logical
|
|
artifact can be frozen so a later publish does not replace the canonical
|
|
version until an operator unlocks it.
|
|
|
|
There is an additional semantic mismatch in treating a locked artifact as a
|
|
simple omission. Narratio publishes immutable run-scoped objects and advances a
|
|
current commit pointer; it does not overwrite a stable artifact object in
|
|
place. Omitting a locked artifact from a later commit can make it disappear
|
|
from current-state discovery rather than preserve the locked canonical version.
|
|
|
|
### Approved lock model
|
|
|
|
Use one remote lock object per stable logical artifact source. Its location is:
|
|
|
|
```text
|
|
{session_prefix}/locks/{sha256(normalized_source_id)}.lock
|
|
```
|
|
|
|
The lowercase hexadecimal SHA-256 name provides a fixed, portable object-key
|
|
segment; the bounded object payload is versioned JSON containing the normalized
|
|
source ID, an optional reason, and its creation time. Readers verify that the
|
|
payload source hashes to the object name. The stable artifact source ID, not the
|
|
configured remote destination, is the lock identity. Configuration can change
|
|
a destination; it must not accidentally evade a lock on the logical artifact.
|
|
|
|
Lock object presence is authoritative, subject to successful bounded decoding
|
|
and identity validation of that payload. No central index is authoritative.
|
|
|
|
### Approved changes
|
|
|
|
- Remove static `pipeline.publish.locks`, the mutable central lock-store schema,
|
|
static/remote merging and precedence, shared-document mutation retries, and
|
|
runtime injection of locks into resolved pipeline configuration.
|
|
- Extend the narrow object-store contract with deletion so an individual lock
|
|
object can be removed. Implement and test the operation in each storage
|
|
backend and fake.
|
|
- Keep the existing lock list/add/remove user workflow, adapted to per-source
|
|
objects:
|
|
- list enumerates and validates the lock prefix;
|
|
- add creates one object, fails clearly when it already exists, and permits
|
|
`--force` to replace its non-authoritative reason and creation metadata; and
|
|
- remove deletes that object, treating an already-absent object as an
|
|
idempotent success with a clear informational result.
|
|
- Check each relevant lock before spending work to publish its artifact and
|
|
check again immediately before advancing the current commit pointer.
|
|
- Do not introduce a distributed transaction protocol. Narratio accepts the
|
|
proportionate, documented boundary that a lock created after the final check
|
|
may race the pointer write; its trusted, low-concurrency operating model does
|
|
not justify cross-object transaction machinery.
|
|
|
|
### Locked-artifact carry-forward
|
|
|
|
When building a new remote commit:
|
|
|
|
- an unlocked artifact uses the newly validated and uploaded artifact
|
|
declaration;
|
|
- a locked artifact already present in the prior current commit carries that
|
|
exact prior immutable declaration into the new commit;
|
|
- a locked artifact with no prior current declaration remains absent and is not
|
|
treated as a missing required output; and
|
|
- invalid or unverifiable prior current state fails publication rather than
|
|
silently replacing or dropping an artifact that is supposed to be locked.
|
|
|
|
The initial lock read determines which prior declarations are carried into the
|
|
new immutable commit. If the final recheck finds that an artifact became locked
|
|
after that commit was constructed, publication must not advance the current
|
|
pointer; the unselected immutable attempt may remain, and the next run rebuilds
|
|
the commit with the prior declaration carried forward. If a lock was removed
|
|
after the initial read, the in-progress commit may conservatively carry the old
|
|
declaration once; a later publish can advance it.
|
|
|
|
The remote commit contract and validation must permit a carried declaration to
|
|
reference an immutable object under an earlier run prefix within the same
|
|
campaign/session namespace. Its destination key, checksum, size, storage
|
|
generation, source identity, and originating run identity remain unchanged and
|
|
fully declared. Restore and current-state readers continue to trust only the
|
|
objects explicitly declared by the selected commit; they must not scan mutable
|
|
prefixes to find a carried artifact.
|
|
|
|
Publish metadata and inspection commands should distinguish newly published,
|
|
carried-forward locked, locked-but-never-published, optional missing, and
|
|
unselected artifacts.
|
|
|
|
### Rationale
|
|
|
|
Independent lock objects make the existence check the concurrency model. Adding
|
|
or removing one lock cannot overwrite another lock's state, so a shared YAML
|
|
merge and retry loop are unnecessary. A stable source identity prevents a
|
|
configuration destination change from bypassing the operator's intent.
|
|
|
|
Carrying the prior declaration forward matches the meaning of canonical lock:
|
|
other artifacts may advance while the locked artifact remains exactly the
|
|
version that was current when locked. It preserves coherent immutable snapshots
|
|
without freezing the entire session or copying the old bytes into every new run
|
|
prefix.
|
|
|
|
### Final state
|
|
|
|
Each artifact lock is one independently visible remote object managed by small
|
|
list/add/remove commands. Publish performs a direct lock lookup and a final
|
|
recheck. New commits preserve prior locked artifact declarations while allowing
|
|
unlocked artifacts to advance, with no static lock authority, shared mutable
|
|
lock manifest, precedence rules, or lost-update retry loop.
|
|
|
|
## Finding 6 — Configuration Diagnostics Are Treated as Semantic Contracts
|
|
|
|
### Underlying issue
|
|
|
|
Configuration composition and validation correctly enforce meaningful product
|
|
rules, but maintenance guidance and tests have also tended to preserve exact
|
|
validation order, exact error prose, incidental formatting, and provenance
|
|
detail whether or not a user-facing operation consumes it. These expectations
|
|
increase refactoring cost and can discourage clearer validation structure.
|
|
|
|
The same configuration area also carries deprecated party/player and other
|
|
legacy compatibility branches addressed by Finding 1.
|
|
|
|
### Approved changes
|
|
|
|
Retain these configuration contracts:
|
|
|
|
- strict YAML decoding and rejection of unknown fields;
|
|
- additive imports with no duplicate field ownership;
|
|
- deterministic selection and application of one profile;
|
|
- meaningful explicit empty, false, zero, list, and map overlay behavior;
|
|
- centralized defaults and semantic validation;
|
|
- canonical party and artifact-family resolution;
|
|
- deterministic effective configuration and secret-free digest behavior; and
|
|
- diagnostics that identify the relevant field and, for composition conflicts,
|
|
the conflicting sources well enough for an operator to correct the problem.
|
|
|
|
Do not treat these as durable contracts unless a specific machine-readable
|
|
interface explicitly says otherwise:
|
|
|
|
- the order in which independent validation failures are discovered;
|
|
- complete prose wording or punctuation;
|
|
- incidental formatting of errors;
|
|
- private helper boundaries or validation call order; and
|
|
- per-node provenance retained only because the current implementation can
|
|
produce it.
|
|
|
|
Remove legacy configuration fields and branches under Finding 1. Refactor
|
|
validators and composition code around semantic domains without preserving
|
|
obsolete diagnostics solely to keep exact-string tests unchanged.
|
|
|
|
Tests should prefer typed errors, error categories, field paths, structured
|
|
data, or the smallest stable semantic fragment. Exact full-string assertions
|
|
are appropriate only for documented CLI syntax or an intentionally stable
|
|
machine-consumed contract.
|
|
|
|
### Rationale
|
|
|
|
Strictness and deterministic effective behavior prevent ambiguous execution;
|
|
exact diagnostic choreography does not. Useful errors can remain precise
|
|
without making their full prose a versioned API. This aligns configuration
|
|
tests with the repository testing policy: protect meaningful outcomes while
|
|
allowing legitimate internal reorganization.
|
|
|
|
### Final state
|
|
|
|
Configuration remains strict, deterministic, source-aware where useful, and
|
|
operator-friendly. Validators may be reorganized and diagnostics improved
|
|
without preserving incidental order or wording. Only the canonical party,
|
|
composition, profile, and artifact-family models remain.
|
|
|
|
## Consolidated Target State
|
|
|
|
After the approved work is complete:
|
|
|
|
- Narratio supports one current configuration and state model with no legacy
|
|
runtime fallbacks;
|
|
- the session manifest is the only durable local progress ledger;
|
|
- analyze checkpoints each successful artifact, preserves valid completed work,
|
|
and resumes from failed, stale, or missing work in dependency order;
|
|
- `cleanup` is the final canonical stage and cannot make a committed publish
|
|
appear to have failed;
|
|
- remote locks are independent per-artifact objects, and locked canonical
|
|
artifacts are carried forward across new current snapshots;
|
|
- configuration tests protect semantic rules rather than diagnostic
|
|
choreography;
|
|
- ordinary logs and command summaries provide proportionate diagnostics without
|
|
participating in correctness; and
|
|
- the combined simplification is eligible for a minor release because the
|
|
removed compatibility behavior was already deprecated and removable.
|
|
|
|
## Open Questions
|
|
|
|
None. The policy and target-state decisions required for a later feature
|
|
roadmap and staged implementation plan are resolved by this audit.
|