From fa4f891689fac62784f9905c30752fb565b7c8ce Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 10 Nov 2026 18:41:23 +0000 Subject: [PATCH] Record the library adoption review findings --- docs/roadmap/remediation.md | 312 ++++++++++++++++++++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 docs/roadmap/remediation.md diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..55a5d4d --- /dev/null +++ b/docs/roadmap/remediation.md @@ -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.