Remove completed roadmaps and repair the benchmark link
This commit is contained in:
@@ -19,8 +19,8 @@ failed output command returns both those bytes and an error containing the
|
|||||||
diagnostic text. The capture remains in memory for the duration of the child;
|
diagnostic text. The capture remains in memory for the duration of the child;
|
||||||
the component benchmark compares one 8 MiB write with the same output in
|
the component benchmark compares one 8 MiB write with the same output in
|
||||||
32 KiB writes, forwarding to `io.Discard`. Smaller writes incur buffer-growth
|
32 KiB writes, forwarding to `io.Discard`. Smaller writes incur buffer-growth
|
||||||
allocations. The [workload-specific results](../roadmap/implementation.md#output-chunking-follow-up)
|
allocations. The [output benchmark](../../internal/gotool/performance_test.go)
|
||||||
record total allocated bytes per operation, excluding subprocess overhead and
|
measures total allocated bytes per operation, excluding subprocess overhead and
|
||||||
diagnostic strings. These totals count all allocations, including superseded
|
diagnostic strings. These totals count all allocations, including superseded
|
||||||
buffers. The returned slice retains the final buffer holding the complete output.
|
buffers. The returned slice retains the final buffer holding the complete output.
|
||||||
Peak live memory depends on buffer growth, garbage collection, and other command
|
Peak live memory depends on buffer growth, garbage collection, and other command
|
||||||
|
|||||||
@@ -1,685 +0,0 @@
|
|||||||
# Computational Efficiency Implementation Plan
|
|
||||||
|
|
||||||
**Status:** Stages 1–11 complete; [review remediation](remediation.md) complete.
|
|
||||||
|
|
||||||
## Purpose And Baseline
|
|
||||||
|
|
||||||
This plan turns the [performance roadmap](performance.md) into ordered work for
|
|
||||||
a Luna coding agent. The inspected baseline is commit
|
|
||||||
`ff90510c380b47a23d0ac1cd8c314b1c4366a15b`; this planning change updates only
|
|
||||||
the two roadmap documents. Recheck the worktree and relevant code before each
|
|
||||||
stage because subsequent stages build on earlier changes.
|
|
||||||
|
|
||||||
Execute Stage 1 through Stage 11 in order. Each stage is one focused
|
|
||||||
implementation prompt. Complete its changes, applicable validation, canonical
|
|
||||||
documentation updates, and local commit under the
|
|
||||||
[agent policy](../policy/agents.md), then proceed. The sequence ends with a
|
|
||||||
results report for review. Consumer pilots, hosted operations, and intermediate
|
|
||||||
releases are outside this implementation sequence.
|
|
||||||
|
|
||||||
Read [development.md](../development.md) and its applicable policy guide.
|
|
||||||
Apply the [architecture policy](../policy/architecture.md),
|
|
||||||
[testing policy](../policy/testing.md), and
|
|
||||||
[documentation policy](../policy/documentation.md). Use graph tools for code
|
|
||||||
discovery as instructed by `AGENTS.md`.
|
|
||||||
|
|
||||||
## Selected Design And Scope
|
|
||||||
|
|
||||||
| Area | Selected implementation |
|
|
||||||
| --- | --- |
|
|
||||||
| Child process | A small piped-process helper in `internal/process`, sharing existing command setup and error wrapping. |
|
|
||||||
| Git blobs | One lazy `git cat-file --batch` session per candidate tree, with one request/response exchange at a time. |
|
|
||||||
| Raw file bytes | Caller-owned bytes for regular-file reads; immutable symlink strings cached by object ID. |
|
|
||||||
| Repeated checks | Compact formatting outcomes, parsed Markdown documents, and replacement summaries cached by object ID within their check. |
|
|
||||||
| Artifact hashing | Data-only declarations followed by one existing local preflight verification; retain build hashing and missing-upload verification. |
|
|
||||||
| Go output | Separate status and captured-output methods; transfer completed buffers without copying. |
|
|
||||||
| Runtime version | A standard-library `sync.OnceValue` cache for the fallback, with an immediate injected-version return. |
|
|
||||||
|
|
||||||
These are internal implementation choices. Preserve public facade signatures,
|
|
||||||
configuration, CLI semantics, selected check ordering, callback sequencing,
|
|
||||||
findings, failure causes, publication guards, and delivery recovery. Existing
|
|
||||||
public contracts remain authoritative. New helpers expose only the internal
|
|
||||||
operations needed here. File-size selection and existing supported inputs keep
|
|
||||||
their current semantics.
|
|
||||||
|
|
||||||
The Git session and checker caches belong to one validation operation. Existing
|
|
||||||
sequential check execution remains sufficient for this design. The runtime
|
|
||||||
fallback is immutable process data and uses the standard library's concurrent
|
|
||||||
initialization mechanism.
|
|
||||||
|
|
||||||
## Stage 1: Measure Candidate Content And Validation
|
|
||||||
|
|
||||||
**Owners:** Benchmark files in `internal/validation` and root facade fixtures.
|
|
||||||
|
|
||||||
**Completion:** Complete. Measurement-only benchmarks cover tree metadata,
|
|
||||||
content reads and formatting for 1,000 unique or repeated-ID Go blobs in loose
|
|
||||||
and packed repositories, repeated links to identical 1 MiB Markdown documents,
|
|
||||||
retained content after processing, and a real offline facade `Check`. Fixtures
|
|
||||||
verify committed bytes, unique/repeated object IDs, link counts, and Git pack
|
|
||||||
state. The facade fixture selects Go formatting, local-replacement inspection,
|
|
||||||
and Markdown while using test-owned module and build caches.
|
|
||||||
|
|
||||||
### Baseline Measurement Record
|
|
||||||
|
|
||||||
The baseline is commit `ca571aca4f2fd43798174a6d0d7c5a588b5347c9`, before adding
|
|
||||||
measurement support. Results use Go `go1.26.8 linux/amd64`, Git `2.53.0`, and a
|
|
||||||
Linux x86_64 Intel Core i5-7260U at 2.20 GHz. The command was
|
|
||||||
`go test ./internal/validation . -run '^$' -bench '^BenchmarkPerformance' -benchmem -count=3`.
|
|
||||||
Each candidate-content operation used a fresh empty content cache with tree
|
|
||||||
metadata prepared outside its timer. The loose fixtures had loose objects and
|
|
||||||
no pack; packed fixtures were checked after `git gc --aggressive --prune=now`.
|
|
||||||
The unique Go fixture contains 1,000 distinct, formatted files totaling 49,000
|
|
||||||
bytes. The repeated-ID fixture contains 1,000 paths sharing one 15-byte blob.
|
|
||||||
The Markdown fixture contains two identical 1 MiB documents, a 47,780-byte
|
|
||||||
index with 1,000 repeated links, and 2,144,932 bytes processed per check. The
|
|
||||||
complete-check module contains 906 bytes of Go/module input. Its `GOMODCACHE`
|
|
||||||
and `GOCACHE` are test-owned temporary directories, and module network access
|
|
||||||
is disabled; the build cache is warmed before timing.
|
|
||||||
|
|
||||||
| Measurement | Baseline observations | Final comparison |
|
|
||||||
| --- | --- | --- |
|
|
||||||
| Tree metadata, 1,000 entries | Loose: 27.5–28.2 ms; packed: 25.0–28.1 ms. About 1.28 MB allocated per operation. | Loose: 33.0–35.1 ms; packed: 7.43–15.10 ms. About 1.28 MB allocated. |
|
|
||||||
| Unique-blob reads, 49,000 bytes | Loose: 1.86–1.96 s; packed: 2.57–3.08 s. About 34.57 MB allocated per operation. | Loose: 89.8–126.3 ms; packed: 41.5–61.0 ms. About 530 KB allocated. |
|
|
||||||
| Repeated-ID reads, 15,000 bytes across 1,000 paths | Loose: 2.12–3.00 ms; packed: 3.42–7.75 ms. About 50.7 KB allocated per operation. | Loose: 84.8–99.5 ms; packed: 22.3–22.9 ms. About 480–483 KB allocated; slower than baseline because raw file bytes are deliberately not retained between reads. |
|
|
||||||
| Formatting, 1,000 unique files | Loose: 1.87–2.94 s; packed: 2.12–4.55 s. About 36.9 MB allocated per operation. | Loose: 111.9–114.6 ms; packed: 56.2–59.1 ms. About 2.89 MB allocated. |
|
|
||||||
| Formatting, 1,000 paths sharing one object | Loose: 4.52–12.66 ms; packed: 6.11–8.62 ms. About 1.49 MB allocated per operation. | Loose: 2.58–2.81 ms; packed: 2.07–2.13 ms. About 121 KB allocated. |
|
|
||||||
| Markdown links and parsing | 19.4–21.5 ms for two identical 1 MiB documents and 1,000 links; about 7.67 MB allocated per operation. | 10.90–11.14 ms for the same fixture; about 2.29 MB allocated. |
|
|
||||||
| Facade check, standard Go suite only | 385–690 ms; 906 module bytes; 336–340 KB allocated in the benchmark process. | 374.0–386.9 ms; 906 module bytes; 336–337 KB allocated in the benchmark process. |
|
|
||||||
| Facade check with selected file checks | 528–634 ms; 906 module bytes; 632–646 KB allocated in the benchmark process. | 379.7–391.8 ms; 906 module bytes; 503–513 KB allocated in the benchmark process. |
|
|
||||||
| Retained content after formatting and Markdown parsing | 1,385,040–1,385,616 bytes above the post-GC metadata baseline, with the candidate and Markdown cache kept alive. | 180,384–198,000 bytes above the post-GC metadata baseline, with the same caches kept alive. |
|
|
||||||
|
|
||||||
Raw output is in temporary storage at `/tmp/releasekit-performance-baseline.txt`.
|
|
||||||
The shared machine produced substantial timing variation, especially for packed
|
|
||||||
content and the full check; allocations and retained-byte observations were
|
|
||||||
more stable. `B/op` for facade checks measures allocations in the benchmark
|
|
||||||
process and does not include allocations inside its Go subprocess. These are
|
|
||||||
workload-specific component measurements, not runtime guarantees.
|
|
||||||
|
|
||||||
1. Add benchmarks named with the `BenchmarkPerformance` prefix. Reuse local Git
|
|
||||||
fixture patterns; adapt shared helper parameters to `testing.TB` only where
|
|
||||||
a benchmark needs them.
|
|
||||||
2. Measure content access and formatting for 1,000 unique regular Go blobs.
|
|
||||||
Include loose and packed objects and a repeated-object-ID case. Measure
|
|
||||||
Markdown checking with a 1 MiB document, repeated links to it, and identical
|
|
||||||
contents under different directories.
|
|
||||||
3. Measure a complete facade `Check` using a temporary committed Go module with
|
|
||||||
a valid release note, Go formatting, local replacements, and Markdown selected.
|
|
||||||
Keep its Go suite real and offline. Prepare fixtures outside timed work.
|
|
||||||
Report allocations and bytes processed, separating content access, tree
|
|
||||||
metadata, and Go test/compile work.
|
|
||||||
4. Add one diagnostic retained-memory observation after file processing, with
|
|
||||||
the candidate and document cache kept alive, to expose raw-content retention.
|
|
||||||
Keep GC/profile collection outside timing loops.
|
|
||||||
5. Create a measurement record in this document for baseline and final results.
|
|
||||||
Include commit, Go/Git versions, fixture sizes, packed/loose state, and cache
|
|
||||||
state. Keep raw benchmark output or profiles in temporary storage.
|
|
||||||
|
|
||||||
**Validation and completion:** Run these benchmarks with `-run '^$'`,
|
|
||||||
`-bench '^BenchmarkPerformance'`, `-benchmem`, and `-count=3`. Verify the
|
|
||||||
fixtures inspect the intended bytes. Synthetic modules use test-owned module
|
|
||||||
caches under the developer guide. Timing ratios remain reported measurements;
|
|
||||||
behavioral tests protect content, failures, and ownership. This stage changes
|
|
||||||
measurement support only.
|
|
||||||
|
|
||||||
## Stage 2: Measure Delivery, Output, And Runtime Lookup
|
|
||||||
|
|
||||||
**Owners:** Benchmark files in `internal/delivery`, `internal/gotool`,
|
|
||||||
`internal/runtimeinfo`, and root delivery fixtures.
|
|
||||||
|
|
||||||
**Completion:** Complete. Measurements cover declarations and local
|
|
||||||
verification of two deterministic 16 MiB files, prebuilt facade delivery for
|
|
||||||
fresh uploads and matching retries, combined output capture and forwarding at
|
|
||||||
4 KiB and 8 MiB, and injected and real metadata version access. The local HTTP
|
|
||||||
fixture hashes and counts incoming multipart bytes incrementally and streams
|
|
||||||
matching files from immutable fixture data. It verifies uploaded identities and
|
|
||||||
that retries match all three declared assets without uploading again.
|
|
||||||
|
|
||||||
### Delivery, Output, And Runtime Baseline
|
|
||||||
|
|
||||||
The baseline is commit `29e1bab`, before production refactoring, using the same
|
|
||||||
Go, Git, operating system, processor, benchmark command, and three repetitions
|
|
||||||
recorded above. Artifact declaration and preflight fixtures contain two
|
|
||||||
deterministic 16 MiB files (33,554,432 binary bytes per operation); setup and
|
|
||||||
initial writes are outside the timers. Declaration currently hashes each file,
|
|
||||||
and declaration plus local verification therefore measures both hashing passes.
|
|
||||||
The facade benchmark uses a committed tagged source fixture and prebuilt
|
|
||||||
artifacts, so its time includes local source checks, hashing, HTTP requests, and
|
|
||||||
streamed body comparison or upload, but no Go compilation. Fresh hosted state is
|
|
||||||
reset outside the timed region for every operation. Retry state is prepared
|
|
||||||
before timing. The fixture server retains only digest and size metadata for
|
|
||||||
uploads and streams known matching bytes directly from shared fixture content.
|
|
||||||
|
|
||||||
Output measurements call the combined writer directly, with forwarding to
|
|
||||||
`io.Discard`, then capture its completed buffer. Each baseline and final
|
|
||||||
comparison writes its complete input in one call. The baseline includes the
|
|
||||||
completed-output copy. Runtime fallback measurements call `Version()` repeatedly
|
|
||||||
against real build metadata; injected access measures the selection helper's
|
|
||||||
injected-version branch. The benchmark process reports its own allocations,
|
|
||||||
including local HTTP fixture work; streamed artifact bodies are not buffered by
|
|
||||||
the fixture server.
|
|
||||||
|
|
||||||
| Measurement | Baseline observations | Final comparison |
|
|
||||||
| --- | --- | --- |
|
|
||||||
| Artifact declarations, two 16 MiB files | 94.3–95.4 ms; 33,554,432 bytes processed; about 68.9 KB allocated. | 11.4–19.4 μs; 33,554,432 declared bytes; 2,296 B and 33 allocations. Declaration no longer reads or hashes file contents. |
|
|
||||||
| Declarations plus local verification | 186.7–188.4 ms; 33,554,432 binary bytes per each of two passes; about 136.2 KB allocated. | 94.3–95.3 ms; one 33,554,432-byte verification pass; about 69.7 KB and 58–59 allocations. |
|
|
||||||
| Facade delivery, fresh uploads | 429–447 ms; 32 MiB binary payload, two files plus manifest; 1.43–1.46 MB allocated. | 331.9–415.3 ms; same payload; 1.38–1.39 MB allocated. |
|
|
||||||
| Facade delivery, matching retry | 309.1–310.0 ms; 32 MiB of local verification plus streamed remote comparisons; 1.21–1.22 MB allocated. | 210.7–217.5 ms; same local verification and streamed comparisons; 1.15 MB allocated. |
|
|
||||||
| Combined output, one 4 KiB write and `io.Discard` | 1.82–1.87 μs; 8,256 B and 3 allocations per operation. | 0.94–1.06 μs; 4,176 B and 2 allocations. |
|
|
||||||
| Combined output, one 8 MiB write and `io.Discard` | 2.02–2.36 ms; 16.78 MB and 3 allocations per operation, including the completed capture copy. | 1.73–2.22 ms; about 8.39 MB and 2 allocations. |
|
|
||||||
| Injected version selection | 0.745–0.754 ns; 0 B and 0 allocations. | 0.754–0.758 ns; 0 B and 0 allocations. |
|
|
||||||
| Repeated real metadata fallback | 1.438–1.474 μs; 1,184 B and 7 allocations per operation. | 4.59–4.65 ns after cache initialization; 0 B and 0 allocations. Separate initialization: 1.54–2.14 μs; 1,264 B and 9 allocations. |
|
|
||||||
|
|
||||||
The delivery observations include local HTTP and filesystem costs specific to
|
|
||||||
these fixtures. The output component benchmark does not include child startup;
|
|
||||||
the real delivery benchmark does include HTTP request handling. These results
|
|
||||||
describe measured workloads and do not establish runtime guarantees.
|
|
||||||
|
|
||||||
### Output Chunking Follow-Up
|
|
||||||
|
|
||||||
The R3 follow-up extends `BenchmarkPerformanceCombinedOutput` with 32 KiB
|
|
||||||
writes. Three repetitions on Go 1.26.8, linux/amd64, and the Intel Core i5-7260U
|
|
||||||
used the completed writer at `7d92c47` with the extended benchmark:
|
|
||||||
|
|
||||||
```sh
|
|
||||||
GOPROXY=off GOSUMDB=off GOTOOLCHAIN=local go test ./internal/gotool -run '^$' -bench '^BenchmarkPerformanceCombinedOutput$' -benchmem -count=3 -benchtime=1s
|
|
||||||
```
|
|
||||||
|
|
||||||
| Workload, forwarding to `io.Discard` | Elapsed time | Total allocated bytes/op | Allocations/op | Returned output length |
|
|
||||||
| --- | --- | --- | --- | --- |
|
|
||||||
| One 8 MiB write | 1.49–1.56 ms | 8,388,697–8,388,702 | 2 | 8,388,608 bytes |
|
|
||||||
| 32 KiB writes totaling 8 MiB | 2.56–2.93 ms | 16,744,532–16,744,541 | 10 | 8,388,608 bytes |
|
|
||||||
|
|
||||||
Both workloads verify complete captured and forwarded bytes outside the timed
|
|
||||||
region. Measurements include capture and forwarding to `io.Discard`, excluding
|
|
||||||
subprocess costs and failure diagnostic strings. Total allocated bytes count
|
|
||||||
all allocations during an operation, including superseded buffers. The returned
|
|
||||||
slice retains the final backing buffer; its logical output length is 8 MiB in
|
|
||||||
both cases. Peak live memory was not measured and depends on buffer growth,
|
|
||||||
garbage collection, and other command work. These results describe the measured
|
|
||||||
workloads and establish no numerical runtime guarantee. Both paths transfer the
|
|
||||||
completed buffer without the former final copy.
|
|
||||||
|
|
||||||
### Final Comparison Record
|
|
||||||
|
|
||||||
The final benchmark run measured commit `42569b4`, before this Stage 11 record
|
|
||||||
was committed, using Go `go1.26.8 linux/amd64`, Git `2.53.0`, and Linux x86_64
|
|
||||||
on an Intel Core i5-7260U at 2.20 GHz (`devbench-02`). It used the same
|
|
||||||
`-run '^$' -bench '^BenchmarkPerformance' -benchmem -count=3` boundaries and
|
|
||||||
the same temporary fixtures and cache setup as the baseline. Raw final output
|
|
||||||
is in `/tmp/releasekit-performance-final-stage1.txt`,
|
|
||||||
`/tmp/releasekit-performance-final-delivery.txt`,
|
|
||||||
`/tmp/releasekit-performance-final-output.txt`, and
|
|
||||||
`/tmp/releasekit-performance-final-version.txt`.
|
|
||||||
|
|
||||||
The unique-blob batch reader reduced measured reads and formatting from seconds
|
|
||||||
to tens or hundreds of milliseconds and cut their allocations by roughly an
|
|
||||||
order of magnitude or more. Formatting repeated object IDs also improved in
|
|
||||||
both time and allocations because parsed outcomes are reused. Raw repeated-ID
|
|
||||||
reads regressed: 1,000 loose reads measured 84.8–99.5 ms and 480–483 KB, versus
|
|
||||||
2.12–3.00 ms and 50.7 KB at baseline; packed reads measured 22.3–22.9 ms versus
|
|
||||||
3.42–7.75 ms. This workload rereads small object bodies through the batch
|
|
||||||
session, while regular-file bytes intentionally remain caller-owned and are
|
|
||||||
not cached by the tree. That is the measured time cost of limiting retained
|
|
||||||
source bytes; scoped read reuse could be reconsidered if repeated raw reads
|
|
||||||
become a common workload.
|
|
||||||
|
|
||||||
Retained processed content fell from about 1.39 MB to 180–198 KB with the
|
|
||||||
candidate and parsed Markdown cache kept alive. The Markdown benchmark now
|
|
||||||
allocates about 2.29 MB instead of 7.67 MB and measured 10.90–11.14 ms versus
|
|
||||||
19.4–21.5 ms at baseline. The complete offline facade check with selected file
|
|
||||||
checks measured 379.7–391.8 ms and 503–513 KB in the benchmark process, versus
|
|
||||||
528–634 ms and 632–646 KB at baseline. The Go-suite-only check remains
|
|
||||||
374.0–386.9 ms; its real test/compile subprocess is the main complete-check
|
|
||||||
cost, and benchmark-process allocations exclude subprocess allocations.
|
|
||||||
|
|
||||||
Artifact declarations measured 11.0–11.5 µs and 2,296 B for two 16 MiB files.
|
|
||||||
Declarations plus current local verification measured 92.4–93.3 ms and about
|
|
||||||
69.7 KB, against 186.7–188.4 ms and 136.2 KB at baseline. The current flow
|
|
||||||
therefore removes one 32 MiB local read/hash pass while retaining one local
|
|
||||||
verification before hosted requests. In the prebuilt local-HTTP facade fixture,
|
|
||||||
fresh uploads measured 331.9–415.3 ms and matching retries 210.7–217.5 ms. A
|
|
||||||
retry still reads and verifies local assets and compares streamed remote bytes;
|
|
||||||
those fixture and remote-comparison costs remain in the complete workflow.
|
|
||||||
|
|
||||||
For single-write workloads, output capture allocations fell from 8,256 B to
|
|
||||||
4,176 B for 4 KiB, and from about 16.78 MB to 8.39 MB for 8 MiB. The 8 MiB timing ranges overlap, while one
|
|
||||||
allocation and the completed-buffer copy are removed. The returned capture
|
|
||||||
remains caller-owned in memory; a failed output command can also retain its
|
|
||||||
diagnostic string. Warm runtime fallback access measured 4.59–4.65 ns and zero
|
|
||||||
allocations after a separate 1.54–2.14 µs, 1,264 B initialization. The
|
|
||||||
linker-injected selection remains allocation-free.
|
|
||||||
|
|
||||||
Some microbenchmark timings varied substantially between repeated runs on this
|
|
||||||
shared machine; allocations were steadier. The local delivery server and
|
|
||||||
filesystem are workload fixtures, not a hosted service. These figures
|
|
||||||
describe only the recorded toolchain and workloads and do not imply numerical
|
|
||||||
runtime guarantees. Raw repeated-ID read retention and disk-backed large
|
|
||||||
diagnostics remain follow-up options if their measured workload frequency
|
|
||||||
justifies their memory and lifecycle tradeoffs.
|
|
||||||
|
|
||||||
1. Extend the same benchmark prefix to declaration/preflight processing of two
|
|
||||||
deterministic 16 MiB artifact files. Measure prebuilt facade `Deliver`
|
|
||||||
against a local HTTP fixture for fresh uploads and matching retries.
|
|
||||||
Prepare files outside timed work and reset hosted state for each fresh run.
|
|
||||||
Stream binary bodies through the fixture's digest/size checks and serve
|
|
||||||
matching files incrementally so fixture buffering does not obscure allocation
|
|
||||||
improvements. Describe fixture processing costs in the workflow results.
|
|
||||||
2. Benchmark completed Go-output capture with modest output and 8 MiB diagnostics,
|
|
||||||
including forwarding to `io.Discard`. Reuse the combined writer for component
|
|
||||||
measurements and existing child fixtures for any command measurements.
|
|
||||||
3. Measure injected version access and repeated real metadata-fallback access.
|
|
||||||
Preserve benchmark coverage of initialization and repeated access when the
|
|
||||||
implementation later adds caching.
|
|
||||||
4. Record time, allocations, bytes processed, and workload/cache settings in the
|
|
||||||
measurement record. Run the benchmark set with the same flags and repetitions
|
|
||||||
as Stage 1. Verify delivered bytes and matching-retry behavior.
|
|
||||||
|
|
||||||
**Validation and completion:** Measurements are repeatable offline using local
|
|
||||||
collaborators and temporary files. Fixture state changes occur outside timed
|
|
||||||
work where they are setup. These two measurement stages establish the baseline
|
|
||||||
before production refactoring begins. Default tests remain independent of real
|
|
||||||
credentials and mutable external services.
|
|
||||||
|
|
||||||
## Stage 3: Add The Piped Child-Process Boundary
|
|
||||||
|
|
||||||
**Owner:** [internal/process](../../internal/process/process.go) and its tests.
|
|
||||||
|
|
||||||
**Completion:** Complete. `Run` and `StartPiped` share command construction.
|
|
||||||
Piped children expose caller-owned input and output pipes, cancellation, and a
|
|
||||||
single underlying wait/reap while preserving parent-context and exit errors.
|
|
||||||
Tests cover argument, directory, and environment propagation; an exchange that
|
|
||||||
responds before input EOF; synchronized cancellation; startup failure; and a
|
|
||||||
nonzero exit. The package inventory now describes the pipe lifecycle.
|
|
||||||
|
|
||||||
1. Factor existing command construction and command-error wrapping into small
|
|
||||||
private helpers shared by `Run` and a new internal `StartPiped` operation.
|
|
||||||
Preserve arguments, directory, environment overlay/removal, stderr routing,
|
|
||||||
context handling, and wrapped exit errors.
|
|
||||||
2. `StartPiped` accepts an `Invocation` for command settings and stderr; stdin
|
|
||||||
and stdout are supplied by pipes. Return a small process handle containing
|
|
||||||
the input writer and output reader, plus `Cancel` and `Wait` operations.
|
|
||||||
Keep the underlying `exec.Cmd` private. Use a derived cancellable context so
|
|
||||||
the owner can abort a blocked exchange without cancelling its caller.
|
|
||||||
3. Close acquired pipes on setup/start failure. The caller owns input closure,
|
|
||||||
stdout consumption, and one wait/reap after a successful start. `Cancel`
|
|
||||||
interrupts the child; `Wait` retains the current context/exit error semantics.
|
|
||||||
Read stdout to completion before a normal wait. Release the derived context
|
|
||||||
when the process is reaped; abort paths close active pipes before waiting.
|
|
||||||
4. Extend the existing test executable helper for an exchange that returns
|
|
||||||
output before stdin EOF. Test real argument/environment propagation, normal
|
|
||||||
EOF completion, cancellation after a readiness signal, start failure, and
|
|
||||||
a nonzero child exit. Synchronize cancellation with the child instead of
|
|
||||||
depending on sleep duration.
|
|
||||||
|
|
||||||
**Validation and completion:** Run `internal/process` tests. Existing `Run`
|
|
||||||
behavior still passes, and the piped helper supports repeated exchanges followed
|
|
||||||
by deterministic cleanup. Update the process inventory description when needed.
|
|
||||||
|
|
||||||
## Stage 4: Implement The Git Batch Reader
|
|
||||||
|
|
||||||
**Owners:** A focused reader file and tests under `internal/git`, reusing
|
|
||||||
[source.go](../../internal/git/source.go).
|
|
||||||
|
|
||||||
**Completion:** Complete. `BlobReader` lazily starts one local-only
|
|
||||||
`git cat-file --batch` process, serializes request/response exchanges, and
|
|
||||||
returns caller-owned body bytes after validating the object ID, blob type,
|
|
||||||
representable size, and framing. Missing objects produce contextual errors
|
|
||||||
without invalidating a healthy session. Broken exchanges and cancellation close
|
|
||||||
the pipes and reap the child, preserving the original failure, Git stderr, and
|
|
||||||
process causes. Existing standalone blob and committed-file readers remain in
|
|
||||||
place. Tests cover repeated loose and packed reads, empty and binary bodies,
|
|
||||||
missing objects, partial clones, malformed/truncated responses, cancellation,
|
|
||||||
and child failures.
|
|
||||||
|
|
||||||
The process-reuse benchmark used one 9-byte blob. A new reader per read took
|
|
||||||
1.98–2.24 ms and allocated 37.9 KB in 190 allocations per operation; a shared
|
|
||||||
session took 57–82 μs and allocated 425 B in 8 allocations. The fresh-session
|
|
||||||
measurement includes process startup and clean close per read. The shared-session
|
|
||||||
timer includes lazy startup on its first read; final cleanup is outside the
|
|
||||||
timer. This is a fixture-specific measurement, not a subprocess-count contract.
|
|
||||||
|
|
||||||
1. Add an internal `BlobReader` that stores a repository directory and parent
|
|
||||||
context. Create its process lazily on the first content request through
|
|
||||||
`process.StartPiped` with `git cat-file --batch`.
|
|
||||||
2. Use the same child-only `GIT_NO_LAZY_FETCH=1` and `GIT_ALLOW_PROTOCOL=`
|
|
||||||
settings as `readLocalObjects`. Keep existing standalone `ReadBlob`,
|
|
||||||
`ReadCommitFile`, and batched metadata functions for their current callers.
|
|
||||||
3. Implement `Read(ctx, objectID) ([]byte, error)` as one serialized exchange:
|
|
||||||
validate the ID using the existing helper, write its newline-terminated
|
|
||||||
request, read the response header, validate object/type/size, read exactly
|
|
||||||
the declared bytes, and consume the framing newline. Compare hexadecimal
|
|
||||||
IDs without narrowing the existing accepted case conventions.
|
|
||||||
4. Allocate the response body once and transfer it to the caller. Use buffered
|
|
||||||
header reading and `io.ReadFull`; blob contents can contain arbitrary bytes
|
|
||||||
and newlines. Read only requested bodies and avoid whole-session buffering.
|
|
||||||
Size conversion errors reflect representability on the host.
|
|
||||||
5. Treat Git's missing-object response as a contextual inspection error. Preserve
|
|
||||||
stderr and process causes when the child fails. A broken protocol or I/O
|
|
||||||
exchange aborts that session and reaps the child; return the original cause.
|
|
||||||
6. Honor the read context during blocked I/O, for example by registering
|
|
||||||
`context.AfterFunc` to cancel the child and stopping that registration after
|
|
||||||
the exchange. Coordinate callback completion when the stop function reports
|
|
||||||
it has begun; stopping alone does not wait for that callback. If cancellation
|
|
||||||
races completion, preserve its cause and abort the session. Read captured
|
|
||||||
stderr after reaping to avoid writer races.
|
|
||||||
7. Add idempotent `Close`: an unused reader has nothing to close; a healthy
|
|
||||||
reader closes stdin, finishes stdout consumption, and waits; an aborted
|
|
||||||
reader closes pipes, cancels, and waits. Preserve unexpected exit failures;
|
|
||||||
deliberate abort cleanup retains the original read failure.
|
|
||||||
|
|
||||||
**Validation and completion:** Test repeated reads from loose and packed Git
|
|
||||||
objects, empty/binary bodies, and missing objects. Reuse the partial-clone test
|
|
||||||
approach to verify absent objects cause errors with no transport or object-store
|
|
||||||
population. Use a small child fixture for truncated framing and cancellation
|
|
||||||
mid-response. Assert content and cleanup outcomes; benchmark process reuse
|
|
||||||
without making a subprocess count a public contract. Run Git and process tests.
|
|
||||||
|
|
||||||
## Stage 5: Give Candidate Validation A Scoped Reader
|
|
||||||
|
|
||||||
**Owners:** [tree.go](../../internal/validation/tree.go),
|
|
||||||
[validation.go](../../internal/validation/validation.go), and tree fixtures.
|
|
||||||
|
|
||||||
1. Give `CandidateTree` one lazy `BlobReader` and an internal `Close` method.
|
|
||||||
`ReadCandidateTree` still reads metadata only. Route blob misses through the
|
|
||||||
reader while temporarily retaining the existing byte-cache behavior.
|
|
||||||
2. The shared validation executor owns closure on every return after loading
|
|
||||||
a tree. Complete cleanup before product checks or build callbacks run.
|
|
||||||
Metadata-only selections leave the content reader unstarted.
|
|
||||||
3. Preserve an earlier check's `Failure` and cancellation cause during cleanup.
|
|
||||||
If cleanup reveals an unexpected child failure after successful checks,
|
|
||||||
return a contextual `Failure` attributed to the last content-reading check.
|
|
||||||
Track that check in the executor; normal intentional cancellation during
|
|
||||||
cleanup adds no independent failure classification.
|
|
||||||
4. Update direct tree users in tests and benchmarks to register cleanup or close
|
|
||||||
explicitly. Update synthetic tree fixtures to support lazy reader creation.
|
|
||||||
Keep tree paths, source capture, symlink resolution, and check order intact.
|
|
||||||
|
|
||||||
**Validation and completion:** Run validation, candidate, publication, and facade
|
|
||||||
candidate tests. Cover early check failure and cancellation with cleanup, and
|
|
||||||
preserve workspace/file-size metadata behavior. Re-run content benchmarks with
|
|
||||||
reader startup and closure included in the content-operation timing. Update
|
|
||||||
[validation mechanics](../internal/validation.md) for the landed lifecycle.
|
|
||||||
|
|
||||||
**Completion:** Complete. `CandidateTree` owns a lazy batch reader and exposes
|
|
||||||
idempotent cleanup. The shared executor closes it on every post-load return,
|
|
||||||
before product checks or build callbacks; metadata-only runs leave it unstarted.
|
|
||||||
Earlier check failures and cancellation remain primary, while an unexpected
|
|
||||||
reader exit after successful content checks becomes a contextual failure for
|
|
||||||
the last content-reading check. Direct tree users close their readers. The
|
|
||||||
temporary object-ID byte cache remains until Stage 6. Validation, candidate,
|
|
||||||
and root facade tests passed. Content benchmark timings now include reader
|
|
||||||
startup and close (Go 1.26.8, Git 2.53.0, Linux x86_64 Intel Core i5-7260U,
|
|
||||||
three repetitions): unique reads took 59.8–77.9 ms loose and 22.8–43.0 ms
|
|
||||||
packed; unique-file formatting took 114–188 ms loose and 33.7–55.7 ms packed;
|
|
||||||
repeated-ID reads took 2.18–6.45 ms, repeated-ID formatting took 4.75–6.62 ms,
|
|
||||||
and the Markdown workload took 15.0–16.9 ms. Compared with the Stage 1 timings,
|
|
||||||
these measurements reflect the shared Git process and include its lifecycle;
|
|
||||||
timing varies on the shared machine.
|
|
||||||
|
|
||||||
## Stage 6: Release One-Use Source Bytes And Reuse Formatting Results
|
|
||||||
|
|
||||||
**Owners:** `internal/validation/tree.go`, `validation.go`, and destination tests.
|
|
||||||
|
|
||||||
1. Remove the tree's general raw-blob cache. A regular-file `ReadFile` returns
|
|
||||||
its newly read, caller-owned body without a cache copy. Its GoDoc describes
|
|
||||||
that ownership and the scoped reader; independently repeated raw reads can
|
|
||||||
request the same object again.
|
|
||||||
2. Cache committed symlink targets as owned immutable strings keyed by object
|
|
||||||
ID. Add a private target-reading helper used by destination resolution and
|
|
||||||
module-symlink handling. Preserve original target bytes and traversal order.
|
|
||||||
Explicit `ReadFile` of a symlink still returns independently owned bytes.
|
|
||||||
3. In `checkGoFormat`, cache the formatted/unformatted result for successfully
|
|
||||||
inspected objects by object ID. Release source and formatted bytes from
|
|
||||||
retained references after comparing them. Emit a finding at every selected
|
|
||||||
unformatted path, including duplicate-content paths. Parse/read errors still
|
|
||||||
stop with the current path and cause; check cancellation for each entry.
|
|
||||||
4. Adapt tests that construct the old cache directly. Preserve observable
|
|
||||||
isolation: mutating returned bytes cannot alter later validation or reads.
|
|
||||||
Reuse destination-cycle and repeated-symlink tests for shared target strings.
|
|
||||||
|
|
||||||
**Validation and completion:** Run validation tests and formatting/destination
|
|
||||||
benchmarks. Confirm many unique Go files no longer remain as retained source
|
|
||||||
buffers after formatting, and duplicate-content files retain per-path findings.
|
|
||||||
Record the tradeoff for explicit repeated raw reads. Update validation mechanics
|
|
||||||
with the new ownership and reuse choices.
|
|
||||||
|
|
||||||
**Completion:** Complete. Regular-file reads now return independent bytes and
|
|
||||||
are not retained by `CandidateTree`; repeated explicit reads therefore transfer
|
|
||||||
the blob again. The tree caches owned symlink-target strings by object ID, and
|
|
||||||
explicit symlink reads still return independent byte slices. Go formatting
|
|
||||||
caches only successful formatted/unformatted outcomes by object ID, checks
|
|
||||||
cancellation per path, and reports every unformatted path even when contents
|
|
||||||
repeat. Validation tests and the formatting/Markdown destination benchmarks
|
|
||||||
passed. On the same Go 1.26.8, Git 2.53.0, Linux x86_64 Intel Core i5-7260U
|
|
||||||
setup, three repetitions measured unique-file formatting at 75.0–75.9 ms loose
|
|
||||||
and 37.9–42.5 ms packed, repeated-ID formatting at 1.97–2.17 ms, and the
|
|
||||||
Markdown workload at 16.3–17.0 ms. Formatting used about 2.89 MB/op for unique
|
|
||||||
files and 121 KB/op for repeated IDs. The retained-memory observation was
|
|
||||||
129,496 bytes with the candidate and Markdown cache alive, compared with
|
|
||||||
1,385,040–1,385,616 bytes in the baseline fixture. This retains Markdown parser
|
|
||||||
data by path until Stage 7; it is a workload-specific heap observation.
|
|
||||||
|
|
||||||
## Stage 7: Reuse Owned Markdown And Module Results
|
|
||||||
|
|
||||||
**Owners:** [markdown.go](../../internal/validation/markdown.go),
|
|
||||||
[markdown_check.go](../../internal/validation/markdown_check.go), and
|
|
||||||
[local_replacements.go](../../internal/validation/local_replacements.go).
|
|
||||||
|
|
||||||
1. Key parsed Markdown reuse by resolved regular blob object ID. Keep a path
|
|
||||||
lookup when useful for callers, backed by the shared document records.
|
|
||||||
Relative link resolution and findings use the current source path; a shared
|
|
||||||
parsed document stores content-relative locations, links, and anchors only.
|
|
||||||
2. Give retained link destinations and titles their own string storage, using
|
|
||||||
`strings.Clone` at extraction where needed, so parser/source views cannot
|
|
||||||
keep a large raw body alive. Cache parsed records, release the AST and source
|
|
||||||
from retained references, and preserve heading collision rules, image behavior,
|
|
||||||
and use-site locations.
|
|
||||||
3. Refactor module-declaration selection to identify the resolved regular entry
|
|
||||||
before reading its body. After successful parsing, cache a compact list of
|
|
||||||
filesystem replacements by resolved object ID: target string and source line.
|
|
||||||
Own retained target strings and release raw bytes and the parsed syntax tree
|
|
||||||
from retained references.
|
|
||||||
4. Apply each cached replacement summary to the current declaration path when
|
|
||||||
constructing findings. Resolve each symlink declaration in its own path
|
|
||||||
context. On read or parse failure, preserve the current declaration's path
|
|
||||||
and underlying cause; preserve findings order and versioned-replacement rules.
|
|
||||||
|
|
||||||
**Validation and completion:** Run Markdown/module and facade validation tests.
|
|
||||||
Add behavioral cases for identical Markdown in different directories and two
|
|
||||||
module declarations resolving to one object, including multiple findings with
|
|
||||||
correct paths/lines. Reuse parser and boundary/cycle coverage. Re-run
|
|
||||||
large-document and repeated-content benchmarks and retained-memory observations.
|
|
||||||
Update the validation document's cache description to match implemented ownership.
|
|
||||||
|
|
||||||
**Completion:** Complete. Parsed Markdown documents are shared by regular blob
|
|
||||||
object ID with path lookups; destinations and titles own their string storage,
|
|
||||||
and link locations remain content-relative while resolution uses each current
|
|
||||||
source path. Module declarations resolve to a regular committed entry before
|
|
||||||
reading; successfully parsed filesystem replacements are cached as owned
|
|
||||||
target/line summaries by body object ID and applied to every declaration path.
|
|
||||||
Validation, candidate, and facade tests passed, including shared Markdown
|
|
||||||
resolved from distinct directories and two module symlinks receiving the
|
|
||||||
correct findings at both paths and lines. Three large-document runs measured
|
|
||||||
10.85–11.22 ms and 2.29 MB/op for two identical 1 MiB documents and 1,000
|
|
||||||
repeated links. Three retained-memory observations measured 110,568–120,280
|
|
||||||
bytes with the candidate and parsed Markdown cache alive. These fixture results
|
|
||||||
include the retained per-document links, anchors, and candidate metadata; they
|
|
||||||
are workload-specific observations on the recorded shared machine.
|
|
||||||
|
|
||||||
## Stage 8: Consolidate Delivery Preflight Hashing
|
|
||||||
|
|
||||||
**Owners:** [delivery declarations](../../internal/delivery/assets.go),
|
|
||||||
[delivery orchestration](../../internal/delivery/delivery.go), and their tests.
|
|
||||||
|
|
||||||
1. Make `declaredBuildAssets` perform structural declaration checks, stat files,
|
|
||||||
reuse the recorded artifact SHA-256, and construct expected assets and the
|
|
||||||
manifest. Remove its file-opening/hash pass. Keep target, filename, regular
|
|
||||||
file, declared digest format, and manifest validation at the appropriate
|
|
||||||
existing owner.
|
|
||||||
2. Keep one `gitea.VerifyLocalAssets` call in `Deliver` after declaration and
|
|
||||||
build-identity checks and before hosted requests. This verifies actual bytes
|
|
||||||
and sizes against recorded identities for both managed and prebuilt assets.
|
|
||||||
3. Keep builder hashing, CLI receipt verification, and missing-upload preflight
|
|
||||||
in their current phases. Managed delivery reuses its fresh build metadata for
|
|
||||||
declarations; prebuilt delivery reuses its caller's recorded digests only as
|
|
||||||
expected identities until current bytes have been verified.
|
|
||||||
4. Move the declaration helper's tampering assertion to delivery preflight's
|
|
||||||
behavioral owner. Verify same-size changed contents fail before any hosted
|
|
||||||
request. Reuse source-only, retained-prebuilt, managed cleanup, conflict,
|
|
||||||
partial-upload, and retry coverage.
|
|
||||||
|
|
||||||
**Validation and completion:** Run delivery, Gitea, facade delivery, and CLI
|
|
||||||
receipt/delivery tests. Re-run artifact and complete delivery benchmarks. A fresh
|
|
||||||
missing binary has three hash passes including a managed build, or two within
|
|
||||||
prebuilt delivery; these are explanatory measurements of the chosen flow.
|
|
||||||
Streaming upload, exact remote comparison, and later upload verification remain
|
|
||||||
intact. Update the inventory's delivery mechanics where needed.
|
|
||||||
|
|
||||||
**Completion:** Complete. `declaredBuildAssets` now performs target/name,
|
|
||||||
filename, regular-file, and stat checks, then uses each recorded SHA-256 to
|
|
||||||
build expected assets and the manifest. The separate local verification in
|
|
||||||
`Deliver` remains after build identity checks and before hosted requests; it
|
|
||||||
checks current bytes and sizes for managed and prebuilt artifacts. A same-size
|
|
||||||
prebuilt mutation now fails at this delivery preflight with zero hosted
|
|
||||||
requests. Delivery, Gitea, facade, and CLI tests passed. On the recorded Go/Git
|
|
||||||
and machine setup, three artifact runs measured declarations at 11.3–15.4 µs
|
|
||||||
(2,296 B/op) and declarations plus one local verification at 121–136 ms
|
|
||||||
(about 69.7 KB/op). The Stage 2 comparison was 94.3–95.4 ms and 186.7–188.4 ms
|
|
||||||
respectively, each including the declaration hash pass; the new second result
|
|
||||||
therefore removes one 32 MiB file-read/hash pass from this workload. Three
|
|
||||||
facade runs measured fresh uploads at 485–520 ms and matching retries at
|
|
||||||
214–311 ms. These complete-workflow timings overlap the recorded baselines
|
|
||||||
within shared-machine variation and remain dominated by the local HTTP fixture,
|
|
||||||
upload, and remote comparison work.
|
|
||||||
|
|
||||||
## Stage 9: Transfer Completed Go Output Without Copies
|
|
||||||
|
|
||||||
**Owners:** [gotool.go](../../internal/gotool/gotool.go), validation/build callers,
|
|
||||||
and Go tool fixture tests.
|
|
||||||
|
|
||||||
1. Give `Tool` a status-returning `Run(...args) error` and an output-returning
|
|
||||||
`Output(...args) ([]byte, error)`, with one private execution path. Keep their
|
|
||||||
existing directory, mode, target, writer, and argument inputs.
|
|
||||||
2. On completion, a status-only success releases its capture. An output-consuming
|
|
||||||
call receives the completed buffer's backing bytes with ownership transferred
|
|
||||||
from the private writer. Replace `combinedWriter.Bytes` copying with a small
|
|
||||||
synchronized ownership-transfer operation used after child output has finished.
|
|
||||||
3. Preserve complete failure diagnostics, original causes, combined-output
|
|
||||||
synchronization, and forwarding errors. If `Output` fails, return its captured
|
|
||||||
bytes and diagnostic error as before. Status failures format the same useful
|
|
||||||
diagnostics without creating an unnecessary intermediate snapshot.
|
|
||||||
4. Convert validation test/vet/compile and artifact-build calls to status-only
|
|
||||||
`Run`. Use `Output` for compiler identification. Adapt preparation failure
|
|
||||||
formatting to the same completed-buffer ownership discipline, preserving its
|
|
||||||
network-capable environment. Leave configuration discovery's separate stdout
|
|
||||||
JSON and stderr handling intact.
|
|
||||||
5. Keep full capture in memory. Measure remaining large-log retention and record
|
|
||||||
the disk-backed alternative as follow-up evidence, as scoped by the roadmap.
|
|
||||||
|
|
||||||
**Validation and completion:** Extend existing process/Go fixtures for successful
|
|
||||||
capture, large failure output, cancellation, and a failing output writer. Verify
|
|
||||||
forwarded bytes and meaningful error content/cause. Run Go tool, validation,
|
|
||||||
build, preparation, and affected CLI tests. Re-run output benchmarks and update
|
|
||||||
[Go execution mechanics](../internal/gotool.md).
|
|
||||||
|
|
||||||
**Completion:** Complete. `Tool.Run` now returns status only, while `Tool.Output`
|
|
||||||
returns combined stdout/stderr with ownership of the completed capture buffer;
|
|
||||||
the shared private execution path preserves settings and arguments. Status
|
|
||||||
success drops its capture, and both status and output failures retain complete
|
|
||||||
diagnostics and underlying process or output-writer causes. Validation and
|
|
||||||
artifact compilation use `Run`; compiler identification uses `Output`.
|
|
||||||
Preparation retains its network-capable environment and formats diagnostics
|
|
||||||
from the transferred buffer. Go tool fixtures cover forwarded output, large
|
|
||||||
failure output, cancellation, and writer failure. Go tool, validation, build,
|
|
||||||
facade, preparation, and CLI tests passed. The single-write combined-output
|
|
||||||
benchmark now measures 1.0–1.1 µs and 4,176 B/2 allocations for 4 KiB, and 1.51–1.56 ms and
|
|
||||||
8,388,699–8,388,703 B/2 allocations for 8 MiB. The Stage 2 8 MiB result was
|
|
||||||
2.02–2.36 ms and about 16.78 MB/3 allocations, including its capture copy.
|
|
||||||
The returned 8 MiB slice remains caller-owned in memory; failed Output also
|
|
||||||
creates diagnostic text. Disk-backed diagnostics remain a measured follow-up
|
|
||||||
option because avoiding their additional lifecycle and cleanup work has not
|
|
||||||
been justified here.
|
|
||||||
|
|
||||||
## Stage 10: Cache The Runtime Metadata Fallback
|
|
||||||
|
|
||||||
**Owners:** [runtimeinfo/version.go](../../internal/runtimeinfo/version.go) and
|
|
||||||
runtime/buildinfo tests.
|
|
||||||
|
|
||||||
1. Preserve `injectedVersion` and its linker identity. `Version` checks and
|
|
||||||
returns injection first, then calls a fallback accessor created with
|
|
||||||
`sync.OnceValue`.
|
|
||||||
2. Keep the existing pure `version(injected, readBuildInfo)` selection helper.
|
|
||||||
Add a small private fallback factory accepting the metadata reader and
|
|
||||||
returning the cached accessor. Production supplies `debug.ReadBuildInfo`;
|
|
||||||
tests create independent cache instances with controlled metadata.
|
|
||||||
3. Reuse selection tests. Add repeated/concurrent access coverage with a
|
|
||||||
counting reader to verify reuse and stable results. Test unavailable and
|
|
||||||
development metadata fallback through isolated instances. Keep shared
|
|
||||||
production state immutable during tests.
|
|
||||||
|
|
||||||
**Validation and completion:** Run runtime/buildinfo tests, the existing consumer
|
|
||||||
build-and-run injection test, and focused race validation of the fallback cache.
|
|
||||||
Run facade/dependency checks. Re-run version benchmarks after initializing the
|
|
||||||
cache and report initialization separately from repeated access. The runtime
|
|
||||||
facade and its dependency boundary retain their existing public shape.
|
|
||||||
|
|
||||||
**Completion:** Complete. `Version` returns the linker-injected value before
|
|
||||||
touching fallback state, and a private factory wraps the existing pure
|
|
||||||
selection helper with `sync.OnceValue`. A concurrent counting-reader test
|
|
||||||
confirms one metadata read and stable results; isolated unavailable and
|
|
||||||
development cases continue to resolve to `dev`. Runtime, buildinfo, facade
|
|
||||||
dependency, consumer executable, and focused race tests passed. Three version
|
|
||||||
benchmark repetitions measured injected selection at 0.75–1.03 ns with no
|
|
||||||
allocations, fallback initialization at 1.53–1.74 µs with 1,264 B/9 allocations,
|
|
||||||
and warmed production fallback access at 4.54–4.91 ns with no allocations.
|
|
||||||
The original uncached metadata call measured 1.438–1.474 µs and 1,184 B/7
|
|
||||||
allocations per access. The internal package inventory now records the cached
|
|
||||||
fallback behavior.
|
|
||||||
|
|
||||||
## Stage 11: Verify The Complete Implementation And Record Results
|
|
||||||
|
|
||||||
**Owners:** Affected tests and canonical internal documentation; roadmap status.
|
|
||||||
|
|
||||||
1. Run the complete offline test suite and compiled facade/dependency checks
|
|
||||||
using the [developer guide](../development.md). Resolve remaining failures
|
|
||||||
within this performance scope. Reuse targeted race results where code is
|
|
||||||
unchanged; broaden them only for a concrete remaining concurrency risk.
|
|
||||||
2. Run the Stage 1 and Stage 2 benchmark set again with comparable toolchain,
|
|
||||||
fixtures, cache state, repetitions, and measurement boundaries. Compare
|
|
||||||
component and complete `Check`/`Deliver` results. Record memory and explicit
|
|
||||||
reread tradeoffs as well as time and allocation improvements.
|
|
||||||
3. Record final results alongside the baseline in this document. Explain the
|
|
||||||
remaining Go compilation/test cost, remote-comparison work, and diagnostic
|
|
||||||
retention where measurements show they matter. Separate measured effects
|
|
||||||
from expectations and include machine/workload limitations.
|
|
||||||
4. Check canonical internal docs describe landed reader lifetime, source/result
|
|
||||||
ownership, verification flow, output ownership, and runtime reuse. Preserve
|
|
||||||
existing public contracts; update a public document only if its explanation
|
|
||||||
became inaccurate. Check local links and prose whitespace.
|
|
||||||
5. Mark each stage's actual completion, update the feature roadmap status, and
|
|
||||||
commit the completed work under repository policy. End with a concise report
|
|
||||||
of changes, tests, performance results, and remaining follow-up opportunities.
|
|
||||||
Retain these planning documents with their results for the final review.
|
|
||||||
|
|
||||||
**Completion evidence:** All five target improvements are implemented, relevant
|
|
||||||
existing behavior passes, the public facade checks pass, comparable measurements
|
|
||||||
are recorded, and the worktree is clean. Completion depends on observed behavior
|
|
||||||
and reviewed diffs; benchmarks establish improvements within their measured
|
|
||||||
scope without adding numerical runtime guarantees.
|
|
||||||
|
|
||||||
**Completion:** Complete. The full offline suite (`go test ./...`) and compiled
|
|
||||||
facade/dependency check (`go run ./internal/apicheck`) passed. Focused race
|
|
||||||
validation passed for the runtime fallback, Go output adapter, and process
|
|
||||||
boundary. Targeted fixtures also cover cancellation and writer-failure causes.
|
|
||||||
The canonical internal docs describe Git reader lifetime and source ownership,
|
|
||||||
validation reuse and delivery verification, Go output ownership, and runtime
|
|
||||||
fallback reuse. Local links and Markdown whitespace were checked. All stages
|
|
||||||
are marked complete; this results document is retained for review.
|
|
||||||
|
|
||||||
## Open Questions
|
|
||||||
|
|
||||||
None required for this implementation. The plan selects scoped batch reading,
|
|
||||||
result-based reuse, preserved verification boundaries, in-memory diagnostics,
|
|
||||||
and cached runtime fallback. Disk-backed diagnostics remain a follow-up option
|
|
||||||
whose benefit and lifecycle costs can be assessed from the recorded output
|
|
||||||
measurements.
|
|
||||||
@@ -1,122 +0,0 @@
|
|||||||
# Computational Efficiency Roadmap
|
|
||||||
|
|
||||||
**Status:** All five target components implemented. The
|
|
||||||
[implementation record](implementation.md) owns completed stages and performance
|
|
||||||
results; the [remediation record](remediation.md) owns review findings and their
|
|
||||||
completion status.
|
|
||||||
|
|
||||||
## Objective And Ownership
|
|
||||||
|
|
||||||
Reduce the time, allocations, retained memory, and repeated I/O of routine
|
|
||||||
release operations. Improvements belong behind the existing `releasekit` and
|
|
||||||
`buildinfo` facades and follow the [architecture policy](../policy/architecture.md)
|
|
||||||
and [routine-release priorities](../adr/0002-routine-releases.md).
|
|
||||||
|
|
||||||
This document owns the desired target state and its rationale. The
|
|
||||||
[implementation plan](implementation.md) owns stages, tasks, validation, and
|
|
||||||
implementation status. The [internal inventory](../internal/overview.md) and
|
|
||||||
focused internal documents own implemented mechanics; Go declarations and
|
|
||||||
GoDoc own public contracts.
|
|
||||||
|
|
||||||
## Review Evidence
|
|
||||||
|
|
||||||
The computational efficiency review examined commit `db9e24f` on 2026-10-09.
|
|
||||||
Temporary offline probes produced these local measurements:
|
|
||||||
|
|
||||||
| Probe | Result |
|
|
||||||
| --- | --- |
|
|
||||||
| Read 1,000 unique committed Go blobs totaling 59,670 bytes through the existing path | 1.83–2.00 seconds across three runs. |
|
|
||||||
| Read and verify the same blobs through one Git batch command | 48–59 milliseconds across three runs. |
|
|
||||||
| Read a cached 1 MiB blob | One allocation of 1 MiB per read. |
|
|
||||||
| Resolve runtime version through the Go build metadata fallback in a test executable | Approximately 2 microseconds, 1,184 allocated bytes, and seven allocations per call. |
|
|
||||||
| Resolve an injected runtime version | Zero allocations. |
|
|
||||||
|
|
||||||
Blob timings cover content access, with tree metadata discovery measured
|
|
||||||
separately. They exclude Go tests, compilation, formatting, and Markdown parsing.
|
|
||||||
The fixture contained small, loose Git objects. These are component measurements
|
|
||||||
on one machine; overall release gains depend on consumer workloads. The probes
|
|
||||||
were temporary and left the repository unchanged.
|
|
||||||
|
|
||||||
Static review also identified repeated artifact hashing and unconditional Go
|
|
||||||
output copying. The [implementation record](implementation.md) contains their
|
|
||||||
baseline and completed-implementation measurements.
|
|
||||||
|
|
||||||
## Desired Target State
|
|
||||||
|
|
||||||
### Git Content Access
|
|
||||||
|
|
||||||
Selected file checks share an operation-scoped Git content reader. Requests
|
|
||||||
remain lazy, use committed object IDs, and consume responses incrementally.
|
|
||||||
Subprocess startup is amortized across the operation. Metadata-only checks
|
|
||||||
continue to use the captured tree without starting a content reader.
|
|
||||||
|
|
||||||
The reader preserves offline object access, cancellation, contextual inspection
|
|
||||||
errors, and explicit process cleanup. Each validation operation owns its reader
|
|
||||||
and reusable data independently.
|
|
||||||
|
|
||||||
### Validation Memory And Reuse
|
|
||||||
|
|
||||||
One-use regular-file bytes belong to their immediate consumer and are released
|
|
||||||
from retained references after inspection. Repeated work reuses immutable parsed
|
|
||||||
or compact results keyed by object ID within the relevant check. Reusable
|
|
||||||
symlink targets belong to the candidate tree. Link checks apply shared document
|
|
||||||
data in each source path's own context, preserving findings, source locations,
|
|
||||||
and relative destinations.
|
|
||||||
|
|
||||||
This replaces retention of every inspected raw blob with data that supports
|
|
||||||
actual repeated work. Explicit repeated raw reads may reread an object through
|
|
||||||
the batch reader. The same bytes can be interpreted separately by different
|
|
||||||
checks; reuse within one interpretation preserves object-ID deduplication.
|
|
||||||
|
|
||||||
### Artifact Verification
|
|
||||||
|
|
||||||
Delivery declarations reuse recorded digests and collect file metadata. One
|
|
||||||
preflight verification checks current local bytes before hosted requests. This
|
|
||||||
combines adjacent declaration-time and asset-verification hash passes for both
|
|
||||||
managed and prebuilt delivery.
|
|
||||||
|
|
||||||
Build-result hashing and upload preflight retain their separate responsibilities.
|
|
||||||
Prebuilt artifacts remain caller-owned. Exact remote content comparison,
|
|
||||||
targeted reconciliation, additive retries, and partial-delivery outcomes retain
|
|
||||||
their existing semantics under the [Gitea contract](../integrations/gitea.md).
|
|
||||||
|
|
||||||
### Go Command Output
|
|
||||||
|
|
||||||
Status-consuming commands return status and useful failure diagnostics.
|
|
||||||
Output-consuming commands receive ownership of completed captured bytes without
|
|
||||||
an additional full-buffer copy. Forwarded output, synchronization, command
|
|
||||||
environments, cancellation, and failure causes retain their existing behavior.
|
|
||||||
|
|
||||||
Complete diagnostic capture remains in memory for this implementation. Its
|
|
||||||
remaining cost is measured with large output. Disk-backed capture is a possible
|
|
||||||
follow-up when its benefit justifies additional filesystem and lifecycle work.
|
|
||||||
|
|
||||||
### Runtime Version Lookup
|
|
||||||
|
|
||||||
The runtime implementation resolves its immutable build-metadata fallback once
|
|
||||||
and shares that result safely across calls. Linker injection retains precedence
|
|
||||||
and its inexpensive path. Main-module version selection and the development
|
|
||||||
fallback retain the [runtime facade's semantics](../consumers/runtime-version.md).
|
|
||||||
The runtime dependency graph remains small and independent of orchestration.
|
|
||||||
|
|
||||||
## Priorities And Evaluation
|
|
||||||
|
|
||||||
| Priority | Improvement | Expected benefit |
|
|
||||||
| --- | --- | --- |
|
|
||||||
| High | Batch selected Git blob reads | Amortize subprocess startup and object access across many files. |
|
|
||||||
| High | Clarify file-content ownership and selective reuse | Reduce copies and memory retained across validation checks. |
|
|
||||||
| Medium | Consolidate artifact hashing | Reduce repeated full-file reads and SHA-256 calculations. |
|
|
||||||
| Lower | Avoid unnecessary Go-output copies | Reduce allocation and peak memory for commands with large diagnostics. |
|
|
||||||
| Low | Cache runtime version fallback | Reduce repeated metadata parsing when consumers call the accessor frequently. |
|
|
||||||
|
|
||||||
Evaluate component and complete-workflow gains with offline local fixtures,
|
|
||||||
including unique and repeated object IDs, packed objects, large files, fresh
|
|
||||||
delivery, and retries. Record workload descriptions, allocations, elapsed time,
|
|
||||||
and retained-memory observations. Timings inform implementation and remain
|
|
||||||
measurements specific to the workload and machine.
|
|
||||||
|
|
||||||
Preserve the efficient foundations already present: batched tree metadata,
|
|
||||||
parsed Markdown reuse, streamed artifact I/O, one selected Go test suite, native
|
|
||||||
artifact reuse for version smoke checks, and source-only delivery without
|
|
||||||
compilation. Publication guards and exact content comparisons retain their
|
|
||||||
existing responsibilities throughout this work.
|
|
||||||
@@ -1,150 +0,0 @@
|
|||||||
# PromptKit Bug Report: v0.2.0 Module Checksum Mismatch
|
|
||||||
|
|
||||||
**Status:** Stale local module metadata confirmed; test isolation fixed.
|
|
||||||
Shared-cache recovery and PromptKit's Stage 1 adoption retry succeeded.
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
PromptKit's adoption attempt encountered stale `go.mod` metadata in the local
|
|
||||||
Go module cache. Its checksum differs from the checksum database's recorded
|
|
||||||
value, so Go stops with a security error. Fresh downloads through both the
|
|
||||||
public proxy and the Git origin verify successfully. The published v0.2.0
|
|
||||||
metadata agrees with the checksum database.
|
|
||||||
|
|
||||||
## PromptKit Context
|
|
||||||
|
|
||||||
PromptKit is adopting ReleaseKit v0.2.0 as a Go `tool` dependency, following the
|
|
||||||
release's documented module-tooling workflow. The version tag currently points
|
|
||||||
to commit
|
|
||||||
[`e030794969fb7787e647a90bbc4eb81bd0610f07`](https://gitea.maximumdirect.net/eric/ReleaseKit/commit/e030794969fb7787e647a90bbc4eb81bd0610f07).
|
|
||||||
The attempt used Go 1.26.8 with `GOTOOLCHAIN=local`.
|
|
||||||
|
|
||||||
## Original Consumer Reproduction
|
|
||||||
|
|
||||||
From a Go module using the affected cache, run:
|
|
||||||
|
|
||||||
```sh
|
|
||||||
GOTOOLCHAIN=local go mod download -json gitea.maximumdirect.net/eric/ReleaseKit@v0.2.0
|
|
||||||
```
|
|
||||||
|
|
||||||
The affected cache produces:
|
|
||||||
|
|
||||||
```text
|
|
||||||
verifying gitea.maximumdirect.net/eric/ReleaseKit@v0.2.0/go.mod: checksum mismatch
|
|
||||||
downloaded: h1:KmH607Vc/RK/zKrCB96cWh0QZYvkbxBFvtqV+BkKaxs=
|
|
||||||
go.sum: h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=
|
|
||||||
|
|
||||||
SECURITY ERROR
|
|
||||||
This download does NOT match an earlier download recorded in go.sum.
|
|
||||||
```
|
|
||||||
|
|
||||||
The recorded checksum is visible in the
|
|
||||||
[Go checksum database lookup](https://sum.golang.org/lookup/gitea.maximumdirect.net/eric/!release!kit@v0.2.0).
|
|
||||||
It records `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=` for the module's
|
|
||||||
`go.mod`. The consumer's diagnostic fetch with `GOSUMDB=off` reported the same
|
|
||||||
v0.2.0 tag commit above, archive checksum
|
|
||||||
`h1:EACELngRel4RiulR+Ors56nMZ5fA9orJDyhPxn6UVe4=`, and `go.mod` checksum
|
|
||||||
`h1:KmH607Vc/RK/zKrCB96cWh0QZYvkbxBFvtqV+BkKaxs=`. Disabling the checksum
|
|
||||||
database was used only to inspect the fetched source; it is not a proposed
|
|
||||||
consumer workaround.
|
|
||||||
|
|
||||||
## Expected Behavior
|
|
||||||
|
|
||||||
The published v0.2.0 module metadata and the checksum database record should
|
|
||||||
agree, so Go can verify and download the release normally.
|
|
||||||
|
|
||||||
## Consumer Impact
|
|
||||||
|
|
||||||
PromptKit's dependency preparation stops before it can reliably select
|
|
||||||
ReleaseKit v0.2.0 as a tool. Go's warning prevents proceeding with the normal
|
|
||||||
integrity check. This does not affect PromptKit's runtime API or behavior; the
|
|
||||||
tool dependency is for maintainer workflows.
|
|
||||||
|
|
||||||
## Investigation Findings
|
|
||||||
|
|
||||||
Investigation on 2026-10-08 used Go 1.26.8 and isolated temporary module caches.
|
|
||||||
The local and authoritative remote tags both identify the commit above.
|
|
||||||
|
|
||||||
| Source | `go.mod` checksum |
|
|
||||||
| --- | --- |
|
|
||||||
| Tagged source, public proxy `.mod`, and module archive's `go.mod` | `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=` |
|
|
||||||
| Checksum database | `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=` |
|
|
||||||
| Affected local cached `.mod` | `h1:KmH607Vc/RK/zKrCB96cWh0QZYvkbxBFvtqV+BkKaxs=` |
|
|
||||||
|
|
||||||
The affected entry is
|
|
||||||
`$GOMODCACHE/cache/download/gitea.maximumdirect.net/eric/!release!kit/@v/v0.2.0.mod`.
|
|
||||||
It contains the original dependency-free module declaration. Its modification
|
|
||||||
time is 2026-10-07 at 23:35 UTC, preceding the published candidate. The cached
|
|
||||||
archive and extracted module contain the correct, dependency-bearing `go.mod`.
|
|
||||||
The exact command that created the stale metadata remains unknown.
|
|
||||||
|
|
||||||
### Download And Recovery Evidence
|
|
||||||
|
|
||||||
- Fresh downloads using `GOPROXY=https://proxy.golang.org` and `GOPROXY=direct`
|
|
||||||
both succeeded with `GOSUMDB=sum.golang.org`, checksum bypass settings cleared,
|
|
||||||
and `GOTOOLCHAIN=local`. Both returned the expected commit and checksums.
|
|
||||||
- Copying the affected cached version into an isolated module cache reproduced
|
|
||||||
the exact consumer checksum error.
|
|
||||||
- Quarantining only the stale `.mod` in that copied cache allowed the same
|
|
||||||
download to succeed with checksum verification enabled.
|
|
||||||
|
|
||||||
These results support continued use of the published v0.2.0 version. The
|
|
||||||
investigation left the affected shared cache and repository contents unchanged.
|
|
||||||
Recovery was initially verified in a copied cache. The subsequent shared-cache
|
|
||||||
recovery and consumer retry are recorded below.
|
|
||||||
|
|
||||||
### Related Test-Isolation Defect
|
|
||||||
|
|
||||||
The workflow tests' [environment helper](../../cmd/releasekit/woodpecker_test.go)
|
|
||||||
disabled checksum verification for synthetic fixtures and inherited the caller's
|
|
||||||
module cache. The version-selection test serves synthetic ReleaseKit versions
|
|
||||||
from a local module proxy.
|
|
||||||
|
|
||||||
Running `TestWoodpeckerTagDeliveryUsesTaggedModuleVersion` with a disposable
|
|
||||||
inherited `GOMODCACHE` passed and left synthetic `v0.2.197` and `v0.2.198`
|
|
||||||
metadata in that cache. This confirmed that the tests could contaminate a
|
|
||||||
developer's module cache. It established a related defect; attribution of the
|
|
||||||
stale `v0.2.0` entry to a particular test or command remains unconfirmed.
|
|
||||||
|
|
||||||
### Test-Isolation Resolution
|
|
||||||
|
|
||||||
Both synthetic-proxy test families now use a shared environment helper that
|
|
||||||
assigns a fresh test-owned `GOMODCACHE` and scopes proxy/checksum settings to
|
|
||||||
their Go subprocesses. Source-bootstrap preparation and execution retain
|
|
||||||
separate caches. Tests that build real ReleaseKit source continue to use their
|
|
||||||
prepared dependencies.
|
|
||||||
|
|
||||||
The existing tagged-version test now supplies a disposable caller cache and
|
|
||||||
asserts that it remains empty after fixture execution. This regression failed
|
|
||||||
before the fix and passed afterward. All Woodpecker workflow tests passed.
|
|
||||||
|
|
||||||
## Recommended Follow-Up
|
|
||||||
|
|
||||||
1. Quarantine the affected local `v0.2.0.mod` entry and retry PromptKit's adoption
|
|
||||||
with normal checksum verification. Retain the quarantined bytes as evidence.
|
|
||||||
2. Record the actual cache recovery and consumer retry outcomes here. Preserve
|
|
||||||
the published v0.2.0 tag and checksums; this incident requires no replacement
|
|
||||||
release.
|
|
||||||
|
|
||||||
## Consumer Recovery Outcome
|
|
||||||
|
|
||||||
On 2026-10-08, PromptKit quarantined only the stale shared-cache
|
|
||||||
`v0.2.0.mod`. The original 58-byte file is retained at
|
|
||||||
`/home/eric/releasekit-cache-quarantine-WtdracPk/v0.2.0.mod`; no archive,
|
|
||||||
extracted module, or unrelated cache entry was removed.
|
|
||||||
|
|
||||||
The retry used Go 1.26.8 with `GOTOOLCHAIN=local`, `GOWORK=off`,
|
|
||||||
`GOSUMDB=sum.golang.org`, and cleared `GONOSUMDB`/`GOPRIVATE` bypass settings.
|
|
||||||
`go mod download -json gitea.maximumdirect.net/eric/ReleaseKit@v0.2.0`
|
|
||||||
succeeded and returned the published commit and expected checksums:
|
|
||||||
|
|
||||||
- Commit: `e030794969fb7787e647a90bbc4eb81bd0610f07`.
|
|
||||||
- Archive: `h1:EACELngRel4RiulR+Ors56nMZ5fA9orJDyhPxn6UVe4=`.
|
|
||||||
- `go.mod`: `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=`.
|
|
||||||
|
|
||||||
PromptKit then successfully added the v0.2.0 Go tool dependency and prepared
|
|
||||||
its module dependencies with checksum verification enabled. Its readonly,
|
|
||||||
offline `go test ./...` run passed under the selected local Go installation;
|
|
||||||
the root package's runtime dependency list contains no ReleaseKit import.
|
|
||||||
This completes the previously blocked Stage 1. The remaining adoption stages
|
|
||||||
are still to be implemented. The published v0.2.0 tag remains unchanged.
|
|
||||||
@@ -1,179 +0,0 @@
|
|||||||
# Performance Implementation Remediation
|
|
||||||
|
|
||||||
**Status:** Complete; R1–R4 addressed.
|
|
||||||
|
|
||||||
**Review baseline:** df064bd, reviewed on 2026-10-09 against the
|
|
||||||
[performance target](performance.md) and
|
|
||||||
[implementation plan](implementation.md).
|
|
||||||
|
|
||||||
## Scope And Architecture
|
|
||||||
|
|
||||||
The five intended improvements are present behind the existing public facades:
|
|
||||||
an internal piped-process boundary and lazy Git batch reader, operation-local
|
|
||||||
validation result caches, consolidated local asset preflight, completed Go-output
|
|
||||||
buffer ownership, and a cached runtime metadata fallback. Public API and
|
|
||||||
dependency checks pass. This review found no added public input limits, release
|
|
||||||
gates, or product restrictions in the implementation commits.
|
|
||||||
|
|
||||||
The findings below cover correctness and documentation gaps in that work.
|
|
||||||
Implement fixes within the existing architecture and preserve the established
|
|
||||||
public contracts. The [agent](../policy/agents.md),
|
|
||||||
[testing](../policy/testing.md), and
|
|
||||||
[documentation](../policy/documentation.md) policies govern the follow-up.
|
|
||||||
|
|
||||||
## R1: Preserve Operation Cancellation Through Reader Shutdown
|
|
||||||
|
|
||||||
**Priority:** High.
|
|
||||||
|
|
||||||
**Owners:** [candidate tree](../../internal/validation/tree.go),
|
|
||||||
[validation executor](../../internal/validation/validation.go), and
|
|
||||||
[Git batch reader](../../internal/git/blob_reader.go).
|
|
||||||
|
|
||||||
CandidateTree.readBlob creates its reader with context.Background().
|
|
||||||
BlobReader.Read registers cancellation for an individual exchange and removes
|
|
||||||
that registration when the exchange finishes. The executor's deferred cleanup
|
|
||||||
then calls tree.Close(), whose stdout drain and process wait have no connection
|
|
||||||
to the validation operation's context.
|
|
||||||
|
|
||||||
Consequently, cancellation while the child is shutting down cannot interrupt
|
|
||||||
that cleanup. Validation can remain blocked after its caller cancels. If the
|
|
||||||
child is subsequently terminated independently, the resulting error reports
|
|
||||||
the process exit and loses the caller's cancellation cause.
|
|
||||||
|
|
||||||
**Evidence:** A temporary offline fixture served valid committed Go content,
|
|
||||||
waited until the executor closed batch stdin, signalled readiness, and stalled
|
|
||||||
before exiting. Cancelling Run at that point left it blocked. After the probe
|
|
||||||
terminated the fixture child, Run returned a Go-format cleanup failure for
|
|
||||||
"signal: killed"; errors.Is(err, context.Canceled) was false. The existing
|
|
||||||
TestRunPreservesCancellationDuringReaderCleanup cancels during an unfinished
|
|
||||||
blob read, so it exercises the per-read callback rather than this shutdown gap.
|
|
||||||
|
|
||||||
**Required outcome:** Associate shutdown with the validation operation context,
|
|
||||||
either through the reader's lifetime context or context-aware cleanup. Preserve
|
|
||||||
individual-read cancellation and the primary failure when a check has already
|
|
||||||
failed. Reap the child on all paths. Use the supplied context for cancellation;
|
|
||||||
this fix does not require a new runtime timeout or input restriction.
|
|
||||||
|
|
||||||
**Validation:** Add a regression synchronized to batch-input EOF, cancel while
|
|
||||||
shutdown is still blocked, and verify completion, child cleanup, and preserved
|
|
||||||
cancellation identity. Retain coverage of read cancellation, successful closure,
|
|
||||||
and preservation of an earlier check failure.
|
|
||||||
|
|
||||||
**Completion:** Complete. The tree retains its operation context for the reader
|
|
||||||
lifetime. An EOF-synchronized regression now verifies shutdown cancellation,
|
|
||||||
cancellation identity, and child reaping; it failed before the fix and passes
|
|
||||||
afterward. Race tests passed for validation, Git, and process packages, including
|
|
||||||
existing read cancellation, successful closure, and primary-failure coverage.
|
|
||||||
|
|
||||||
## R2: Attribute Shutdown Failures To A Check That Read Content
|
|
||||||
|
|
||||||
**Priority:** Medium.
|
|
||||||
|
|
||||||
**Owner:** [validation executor](../../internal/validation/validation.go).
|
|
||||||
|
|
||||||
The lastContentCheck variable is assigned when a content-oriented check is
|
|
||||||
selected, even when that check has no applicable files and makes no content
|
|
||||||
request. Deferred cleanup therefore reports the last selected check rather than
|
|
||||||
the last check that used the reader, diverging from the implementation plan's
|
|
||||||
failure-attribution rule and the completion record's description.
|
|
||||||
|
|
||||||
**Evidence:** A temporary fixture with only go.mod and main.go selected Go
|
|
||||||
formatting and Markdown checking. Only formatting read a blob. The child then
|
|
||||||
exited with code 23 during shutdown, and Run reported "markdown-links".
|
|
||||||
|
|
||||||
**Required outcome:** Track actual reader use by each check and preserve the
|
|
||||||
last check that used content access when a later selected check performs no
|
|
||||||
reads. Keep check ordering and earlier failure attribution intact. Reader-use
|
|
||||||
tracking remains internal implementation bookkeeping.
|
|
||||||
|
|
||||||
**Validation:** Extend the existing unexpected-close-failure fixture with a
|
|
||||||
later selected check that has no applicable files. Verify the earlier reader
|
|
||||||
user receives the cleanup cause and the child exit error remains discoverable.
|
|
||||||
|
|
||||||
**Completion:** Complete. Internal blob-read bookkeeping now identifies the last
|
|
||||||
check that actually used the reader. The extended close-failure fixture preserves
|
|
||||||
Go-format attribution when Markdown has no files, including the child exit code
|
|
||||||
and stderr. The regression failed before the fix and passes afterward; validation
|
|
||||||
race tests and compiled public API and dependency checks passed.
|
|
||||||
|
|
||||||
## R3: Qualify Output Allocation Measurements By Their Workload
|
|
||||||
|
|
||||||
**Priority:** Low.
|
|
||||||
|
|
||||||
**Owners:** [Go execution documentation](../internal/gotool.md),
|
|
||||||
[output benchmark](../../internal/gotool/performance_test.go), and the
|
|
||||||
[implementation results](implementation.md).
|
|
||||||
|
|
||||||
The internal Go-output document says an 8 MiB forwarded capture currently
|
|
||||||
allocates about 8.39 MB per operation. The cited benchmark makes one 8 MiB
|
|
||||||
Write call. Actual command output can arrive in smaller chunks, which incur
|
|
||||||
additional bytes.Buffer growth allocations. The statement needs its workload
|
|
||||||
qualification to describe the measured behavior accurately.
|
|
||||||
|
|
||||||
**Evidence:** Temporary component probes using the current writer and io.Discard
|
|
||||||
measured approximately 8,388,691 B/op and two allocations for one 8 MiB write,
|
|
||||||
versus 16,744,535 B/op and ten allocations for the same total output in 32 KiB
|
|
||||||
writes. These measurements exclude subprocess overhead and failure diagnostic
|
|
||||||
strings. Both paths correctly avoid the former final capture copy.
|
|
||||||
|
|
||||||
**Required outcome:** Identify the single-write workload wherever that figure
|
|
||||||
is quoted. Include a representative chunked capture measurement in the benchmark
|
|
||||||
set and results. Distinguish allocated bytes across an operation from retained
|
|
||||||
output bytes and peak memory. Complete in-memory capture remains the selected
|
|
||||||
design; these findings do not require disk-backed diagnostics or a memory cap.
|
|
||||||
|
|
||||||
**Validation:** Compare equal output lengths across the two chunking patterns
|
|
||||||
and confirm captured bytes and forwarding remain correct. Report allocation
|
|
||||||
results as workload-specific measurements with no numerical runtime guarantee.
|
|
||||||
|
|
||||||
**Completion:** Complete. The benchmark now includes equal 8 MiB output in one
|
|
||||||
write and 32 KiB chunks, with untimed byte-for-byte capture and forwarding checks.
|
|
||||||
Three repetitions passed and measured about 8.39 MB/2 allocations and
|
|
||||||
16.74 MB/10 allocations respectively. Go-output documentation and implementation
|
|
||||||
results qualify the single-write figures and distinguish allocation totals,
|
|
||||||
retained output, and unmeasured peak memory.
|
|
||||||
|
|
||||||
## R4: Reconcile Feature And Implementation Status
|
|
||||||
|
|
||||||
**Priority:** Low.
|
|
||||||
|
|
||||||
**Owner:** [performance roadmap](performance.md).
|
|
||||||
|
|
||||||
The performance roadmap still says "Ready for implementation; implementation
|
|
||||||
pending." The implementation document marks Stages 1–11 complete, and the code
|
|
||||||
contains the five target components. Stage 11 explicitly called for updating the
|
|
||||||
feature roadmap status, and its completion record claims the work is complete.
|
|
||||||
|
|
||||||
**Required outcome:** Update the feature roadmap to distinguish the implemented
|
|
||||||
target components from the outstanding review findings recorded here. Keep
|
|
||||||
remediation status in this document and performance results in the implementation
|
|
||||||
record, consistent with their documentation ownership.
|
|
||||||
|
|
||||||
**Validation:** Check status and navigation across the three roadmap documents
|
|
||||||
against the actual implementation and remediation outcomes.
|
|
||||||
|
|
||||||
**Completion:** Complete. The feature roadmap identifies all five implemented
|
|
||||||
components and links to their implementation results and review remediation.
|
|
||||||
The implementation record links to completed remediation. Status, ownership,
|
|
||||||
and local navigation across all three documents were checked against the code
|
|
||||||
and completed R1–R3 work.
|
|
||||||
|
|
||||||
## Remediation Validation
|
|
||||||
|
|
||||||
- The full offline Go test suite passed after the production fixes.
|
|
||||||
- Compiled public API and dependency checks passed.
|
|
||||||
- Race tests passed for validation, Git, and process packages.
|
|
||||||
- The output benchmarks passed three repetitions, including exact capture and
|
|
||||||
forwarding checks for both chunking patterns.
|
|
||||||
- Changed documentation links, fragments, and whitespace checks passed.
|
|
||||||
|
|
||||||
## Review Validation
|
|
||||||
|
|
||||||
- The full offline Go test suite passed; some unchanged packages used cached
|
|
||||||
results.
|
|
||||||
- The compiled public API and dependency checks passed.
|
|
||||||
- Focused race tests passed for internal/git, internal/process,
|
|
||||||
internal/runtimeinfo, internal/gotool, and internal/validation.
|
|
||||||
- The targeted probes used temporary Go overlays and local child-process
|
|
||||||
fixtures. They reproduced R1 and R2 and measured the workload distinction in
|
|
||||||
R3. No production code or permanent tests were changed during this review.
|
|
||||||
Reference in New Issue
Block a user