Record the library adoption review findings
This commit is contained in:
312
docs/roadmap/remediation.md
Normal file
312
docs/roadmap/remediation.md
Normal file
@@ -0,0 +1,312 @@
|
||||
# Library Adoption Review Findings
|
||||
|
||||
**Reviewed source:** `2844ce7e6a6b881bd4658577fd1177eae729f04d`.
|
||||
**Scope:** All 15 stages in the [implementation plan](implementation.md),
|
||||
compared with the [adoption roadmap](library-adoption.md) and pre-sprint
|
||||
revision `40420a2`.
|
||||
**Status:** Open.
|
||||
|
||||
## Architecture Assessment
|
||||
|
||||
ConfigKit selects and composes the v3 application configuration, including
|
||||
main-relative glossary sources and overlays, and records root and stage
|
||||
resolution. PromptKit loads its bundled catalog, embedded Audita prompts, and
|
||||
operator resources using the selected main's context. One engine supplies
|
||||
stage-specific proposal and validation clients and owns provider admission.
|
||||
CommandKit provides routing, reporting modes, and the standard configuration,
|
||||
resource, request, and smoke commands.
|
||||
|
||||
Audita resolves independent context and protection views for each stage.
|
||||
Preflight checks readiness and coordinates the output session before model
|
||||
work. Processing normalizes once, then the runner chunks once, applies stages
|
||||
in order, and carries recoverable failures into reports. ReleaseKit owns
|
||||
validation, source publication, and asynchronous Linux delivery.
|
||||
|
||||
| Stages | Reviewed behavior |
|
||||
| --- | --- |
|
||||
| 1 and 15 | Dependency and toolchain pins, release profile, launcher, tag verification, and candidate evidence. A disposable ReleaseKit check passed with Go 1.27.3, including race tests, vet, Markdown and module checks, both Linux builds, and version output. The plan records the annotated-tag delivery rehearsal. |
|
||||
| 2–4 | Native application load, main-relative engine inputs, prompt resources and source accounting, prepared role clients, retries and repairs, and redaction. See R2 and R4. |
|
||||
| 5–6 | Native app and PromptKit scopes, typed routing, preview, validation, and explicit smoke, including controlled transport tests. See R5. |
|
||||
| 7–8 | Destination checks, deferred device/pipe delivery, transcript replacement, ordinary reports, optional debug, and reporting modes. See R1, R7, and R9. |
|
||||
| 9–10 | Native identified glossary composition, typed extensions and selections, recorded defaults, and explicit clearing after CLI assignments. |
|
||||
| 11–12 | Stage-specific glossary views, late-stage readiness, fixed chunks, ordered mutation, and partial success. See R3 and R9. |
|
||||
| 13–14 | Stage attribution, report schemas, canonical docs, migration, examples, ADR, and attribution. See R1, R6, R7, and R8. |
|
||||
|
||||
## R1 — P2: Keep Failure Records and Counts Consistent
|
||||
|
||||
**Category**
|
||||
|
||||
Incorrect ordinary reports in Stages 7, 8, and 13.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[Finalization](../../internal/cli/process_output.go#L24) appends
|
||||
run-level output errors after the report snapshot's counts have been computed.
|
||||
A real run with an empty pipeline and `--output /dev/full` returned status 1.
|
||||
Both YAML summaries still showed `run_errors: 0`, while `errors.yml` contained
|
||||
an `output_failure` record. The Markdown report uses the same stale count.
|
||||
[Report initialization](../../internal/cli/process_inputs.go#L89) prepares a
|
||||
failed-status bundle by changing only the status, so a failed initialization
|
||||
can leave reports with no record of the known failure.
|
||||
|
||||
[Pipeline failure attribution](../../internal/cli/process_pipeline.go#L48)
|
||||
also takes the first operation with any failure reason from the last result.
|
||||
A recoverable timeout or unusable decision set can precede the operation
|
||||
that actually stopped processing. This can overwrite the cancellation reason
|
||||
and cite a different chunk or validator. A stage-only origin is also rendered
|
||||
as chunk zero by [report mapping](../../internal/framework/processreport/report.go#L119).
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Build failed reports from the actual run failures and
|
||||
known output failures, deriving counts and records together. Keep fatal
|
||||
classification tied to the error that stopped the runner. Add stage, chunk,
|
||||
and operation attribution only where the runner has that evidence.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
A late transcript or report failure produces consistent
|
||||
failed-status records and counts in the remaining reports. A recoverable error
|
||||
followed by cancellation or a fatal operation retains both facts separately.
|
||||
The primary failure and any already published transcript survive secondary
|
||||
report failures.
|
||||
|
||||
## R2 — P2: Preserve Host Prompt Source Context Through Symlinks
|
||||
|
||||
**Category**
|
||||
|
||||
Main-relative resource loading divergence in Stage 3.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[Engine assembly](../../internal/framework/llm/runtime.go#L58)
|
||||
passes ConfigKit's passive host resource view back as an opaque native
|
||||
filesystem. That view lacks native symlink inspection. In a disposable project,
|
||||
`prompts.yml -> defs/resources.yml` acquired the prompt but resolved its
|
||||
`content_file: message.txt` beside the link, failing despite the file being
|
||||
present beside `defs/resources.yml`. An ordinary manifest import of the same
|
||||
target worked. Adding a file beside the link also made the linked definition
|
||||
load, demonstrating that it used the wrong base.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Carry the recorded host base and containment root into
|
||||
PromptKit as a supported host context, so ConfigKit can acquire host sources
|
||||
and resolve links using its native host rules. Keep embedded/native filesystem
|
||||
inputs separate. If the public handoff cannot express the required context,
|
||||
request the missing upstream facility with this fixture.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
A prompt acquired through a supported in-root host symlink
|
||||
resolves its message file beside the acquired definition. Source evidence and
|
||||
output collision checks still protect consumed files. Preserve the absent-main
|
||||
behavior and the recorded wider-root case.
|
||||
|
||||
## R3 — P2: Separate Glossary Context From Protection in Proposal Prompts
|
||||
|
||||
**Category**
|
||||
|
||||
An unintended prompt restriction in Stage 11.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[The runner's specifications](../../internal/framework/contracts/contracts.go#L101)
|
||||
correctly derive independent rich entries and protected vocabulary. The
|
||||
[proposal builder](../../internal/framework/proposal_generation/module_proposal.go#L35)
|
||||
passes only the rich entries. The embedded [grammar](../../internal/prompts/assets/prompts/modules.grammar.proposal.yaml#L21),
|
||||
[homophones](../../internal/prompts/assets/prompts/modules.homophones.proposal.yaml#L19),
|
||||
and [spoken-word](../../internal/prompts/assets/prompts/modules.spoken_word.proposal.yaml#L23)
|
||||
prompts call those entries protected. With `glossaries: [party]` and
|
||||
`protected_terms: []`, they still tell the model to preserve party spellings.
|
||||
A protection-only selection also has no distinct representation in these prompts.
|
||||
The deterministic checks see the intended independent selection, but they
|
||||
cannot recover valid proposals that the model was instructed to omit.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Describe rich entries as context and give protection
|
||||
instructions the effective protected spellings separately wherever they are
|
||||
needed. Use the same stage-local exemption for glossary correction. Retain
|
||||
the domain limits on each module, including acoustic plausibility and the
|
||||
spoken-word repetition exception.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
Captured requests distinguish context-only and protection-only
|
||||
selections. Explicit empty protection imposes no glossary-derived prompt
|
||||
restriction. The deterministic and apply-time checks still use the effective
|
||||
stage vocabulary.
|
||||
|
||||
## R4 — P2: Show Native Configuration Diagnostics During Processing
|
||||
|
||||
**Category**
|
||||
|
||||
Missing migration guidance and actionable failure detail in
|
||||
Stages 2 and 8.
|
||||
|
||||
**Evidence**
|
||||
|
||||
The [version rule](../../internal/core/config/config.go#L139)
|
||||
provides migration guidance as a ConfigKit diagnostic. The
|
||||
[process error path](../../internal/cli/process.go#L22) prints the load error's
|
||||
summary. A real invocation using a v2 main file returned status 2 with
|
||||
`configkit: configuration load failed with 1 error` and no migration guidance.
|
||||
`config validate --scope app` on that same file printed the path, location,
|
||||
and migration guidance.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Surface the native safe findings for application-load
|
||||
failures in the process command, using CommandKit's normal error presentation
|
||||
and retaining the typed cause. Reuse the native diagnostics rather than
|
||||
maintaining a second configuration validator or formatter.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
Processing an obsolete main file explains the migration before
|
||||
creating outputs or calling a model. An ordinary invalid setting identifies
|
||||
the actionable configuration problem through the same path.
|
||||
|
||||
## R5 — P3: Keep PromptKit Configuration Comparison Sides Independent
|
||||
|
||||
**Category**
|
||||
|
||||
Native behavior overridden in Stage 5.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[PromptKit's loader](../../internal/cli/app_wiring.go#L46) memoizes
|
||||
engines by deep equality of the selection within the invocation state. That
|
||||
state is shared by the native configuration suite. The released ckprompt
|
||||
comparison adapter acquires both sides independently, including equal
|
||||
selections. Audita returns the left-hand capture for an equal right-hand
|
||||
selection, adding a shared-snapshot rule the plan explicitly avoids.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Give the comparison loader independent acquisitions.
|
||||
Keep shared acquisition within a selected processing or request workflow
|
||||
where it serves the captured-engine requirement.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
Each comparison side reaches the loader and gets its own
|
||||
native evidence. Other standard commands continue to use the same source
|
||||
selection rules and native comparison status.
|
||||
|
||||
## R6 — P3: Render Stage IDs as Data in Markdown
|
||||
|
||||
**Category**
|
||||
|
||||
Incomplete output escaping in Stage 13.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[Markdown rendering](../../internal/core/reporting/markdown.go#L174)
|
||||
handles table separators, backticks, newlines, and HTML, but a valid ID such as
|
||||
`[party](https://example.invalid/)` still renders as a link. Emphasis delimiters
|
||||
can similarly hide literal characters. The stage ID is an exact operator label
|
||||
and the same text is preserved in machine records.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Render IDs as literal text within report labels using
|
||||
Markdown-appropriate escaping. Continue to allow arbitrary supported ID text
|
||||
and use generated coordinates for paths.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
A representative ID containing Markdown syntax renders with
|
||||
its literal text, retains the exact YAML/JSON ID, and stays in its intended table
|
||||
cell. Existing unnamed and repeated stage labels remain distinguishable.
|
||||
|
||||
## R7 — P3: Retire Disconnected Cutover Code and Test the Live Paths
|
||||
|
||||
**Category**
|
||||
|
||||
Dead production code and misleading test coverage in Stages 7,
|
||||
8, 13, and 15.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[The old report-directory helper](../../internal/cli/config_input.go)
|
||||
has only a test caller. [Processing](../../internal/cli/process_inputs.go#L72)
|
||||
implements directory selection separately. The former
|
||||
[configuration explanation](../../internal/framework/llm/safe_view.go) and
|
||||
[utilization view](../../internal/framework/processreport/llm.go#L24) also have
|
||||
only test callers. The runner still collects validator timing for the latter.
|
||||
The single `GlossaryPath` survives through report data and prints an empty
|
||||
`Glossary input:` line after the standalone glossary loader was removed.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Put destination-selection coverage on the live path.
|
||||
Remove superseded formatters, bookkeeping, DTO fields, and tests whose only
|
||||
consumer was a retired artifact. Preserve required stage attribution,
|
||||
redaction, and native configuration provenance on their active paths.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
Regression tests exercise actual report directory selection.
|
||||
Remove the stale standalone glossary path from current evidence. Retain
|
||||
diagnostics with an active consumer or a documented requirement.
|
||||
|
||||
## R8 — P3: Clarify the Configuration Cutover and Preflight Documentation
|
||||
|
||||
**Category**
|
||||
|
||||
Documentation divergence in Stage 14.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[The migration guide](../migrations/v3.md) calls the cutover
|
||||
“Audita v3”, although Audita 3.x already shipped with application schema v2.
|
||||
It also omits the change that default-mode processing hides completion
|
||||
counts, including when ordinary reports are disabled.
|
||||
[Subprocess guidance](../integrations/subprocess.md#result-handling) says
|
||||
failed reports can describe invalid input after initialization. Transcript
|
||||
parsing now occurs before report preparation.
|
||||
[The v3.0.0 note](../releases/v3.0.0.md) pins historical examples to its tag
|
||||
but still links the old format description to the current configuration guide.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Distinguish application schema v3 from the executable
|
||||
release version. Explain how to see completion counts at the cutover, link
|
||||
historical format guidance to its applicable revision, and align subprocess
|
||||
guidance with early transcript validation.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
A v3.1.x operator can identify which schema is changing and
|
||||
how to observe completed runs. Current guidance explains which failures can
|
||||
leave reports. Historical references continue to describe their own versions.
|
||||
|
||||
## R9 — P2: Honor Cancellation in Normalization-Only Processing
|
||||
|
||||
**Category**
|
||||
|
||||
Cancellation regression in Stages 8 and 12.
|
||||
|
||||
**Evidence**
|
||||
|
||||
[Processing](../../internal/cli/process.go#L44) lost the
|
||||
post-normalization context check during the cutover. The
|
||||
[runner](../../internal/framework/runner/runner.go#L172) checks for cancellation
|
||||
through stage work, which an empty pipeline never enters. In a disposable
|
||||
normalization-only run, sending SIGINT just after the initial reports appeared
|
||||
returned status 0 and published the transcript.
|
||||
|
||||
**Recommended change**
|
||||
|
||||
Check the invocation context at the processing handoff
|
||||
and before committing a successful result. Use the existing canceled outcome
|
||||
and failed-report finalization for cancellation observed before delivery.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
A controlled cancellation after preparation and before
|
||||
successful delivery of an empty-pipeline run leaves the final transcript
|
||||
untouched and attempts failed-status reports. An uncanceled empty pipeline
|
||||
still produces normalized output without provider calls.
|
||||
Reference in New Issue
Block a user