Record publication recovery algorithm findings
This commit is contained in:
@@ -365,3 +365,134 @@ evidence and publication behavior intact; avoid adding a general UTF-8
|
||||
restriction to previously valid Git publication inputs. Cover endpoint/path
|
||||
round trips and equivalent direct-versus-loaded observations, including a
|
||||
distinct replacement-character path, in focused regression tests.
|
||||
|
||||
## Algorithmic audit scope and evidence
|
||||
|
||||
This audit retains the planned flat record store, complete local discovery,
|
||||
deterministic ordering of selected record paths, versioned JSON format, and
|
||||
exact-tag observation semantics. It does not propose indexes, pagination,
|
||||
automatic retention, or changes to those selected algorithms.
|
||||
|
||||
No unexpected quadratic algorithm was found in the added production code.
|
||||
Classification operates on a fixed number of fields and bounded-length object
|
||||
IDs. Record encoding/decoding uses a fixed number of linear passes over a
|
||||
record. Facade and CLI result conversions are linear in returned entries and
|
||||
displayed data. Full-result discovery necessarily retains the records its API
|
||||
returns. The refinements below concern work outside those necessary operations
|
||||
and one partial-result correctness defect.
|
||||
|
||||
The existing codec, discovery, and classification tests passed in a fresh
|
||||
focused run with Go 1.26.8. Complexity analysis used the implementation and
|
||||
the local Go standard-library source, without benchmarks or real-world
|
||||
performance measurements. Disposable functional probes confirmed the parser
|
||||
cause loss and partial-enumeration result loss. The latter used a temporary
|
||||
source overlay only to make the directory-reader boundary return real entries
|
||||
with a controlled error; repository production and test files were unchanged.
|
||||
|
||||
## R11: Filter directory entries before ordering them — P3
|
||||
|
||||
**Relevant code:** `internal/publish/attempts.go:41` through the subsequent
|
||||
filter and `sort.Strings(paths)`.
|
||||
|
||||
`os.ReadDir` already sorts every directory entry by filename. Discovery then
|
||||
filters for JSON record files and sorts their full paths again. Consequently,
|
||||
an overridden store containing many unrelated files pays to order names that
|
||||
will never appear in the result. This is an implementation choice beyond the
|
||||
planned requirement to order selected records.
|
||||
|
||||
**Complexity:** Let `D` be all directory entries, `R` the selected JSON file
|
||||
candidates, and `B` their total bytes read. Treating name comparisons as unit
|
||||
operations, the current directory ordering has worst-case `O(D log D)` cost
|
||||
before the same `O(B)` record work. Filtering an unsorted enumeration and sorting
|
||||
only selected paths costs `O(D + R log R + B)`. When `R` is bounded and `D`
|
||||
grows, directory processing improves from `O(D log D)` to `O(D)`. When nearly
|
||||
every entry is a record, the planned sorting bound remains unchanged. The
|
||||
second sort currently receives already ordered paths; it is redundant, rather
|
||||
than evidence of a quadratic algorithm.
|
||||
|
||||
**Expected behavior:** Keep complete discovery and deterministic path ordering
|
||||
for attempts and their diagnostics. Unrelated directory entries need no order.
|
||||
|
||||
**Remediation opportunity:** Use the standard `os.File.ReadDir` interface to
|
||||
enumerate without implicit sorting, filter candidates, and order those paths
|
||||
once. Batched reads can also reduce intermediate directory-entry storage from
|
||||
`O(D)` to `O(batch size + R)`, excluding retained result data. Preserve partial
|
||||
enumeration errors as described in R13 and cancellation behavior from R04.
|
||||
Keep this a local enumeration refinement; no public file-count limits or
|
||||
changes to record selection are required.
|
||||
|
||||
## R12: Reject trailing JSON without constructing a discarded value — P3
|
||||
|
||||
**Relevant code:** `internal/publish/record.go:24`, particularly the second
|
||||
`decoder.Decode(&trailing)` and its error handling at lines 34–37.
|
||||
|
||||
After decoding the envelope, the loader decodes another complete JSON value
|
||||
into `any` solely to check whether one exists. A second value always makes the
|
||||
record invalid. Nevertheless, an appended array or object is fully parsed and
|
||||
materialized before being discarded. Only trailing whitespace and EOF are
|
||||
acceptable at that point.
|
||||
|
||||
**Complexity:** For a first envelope of `L` bytes and an appended value of `T`
|
||||
bytes containing `M` nodes, the second decode adds `Theta(T)` parsing work and
|
||||
up to `Theta(M)` object/array storage. Rejecting at the first non-whitespace byte
|
||||
after the envelope needs no second value tree. A standard-library complete-
|
||||
document unmarshal of the already loaded bytes can reject that trailing input
|
||||
during syntax validation before constructing it. This removes avoidable parsing
|
||||
and allocations; the existing whole-file read still has its `Theta(L + T)` I/O
|
||||
and buffer bound. It is not an overall quadratic-to-linear claim.
|
||||
|
||||
There is also a correctness inconsistency: a non-EOF error from that second
|
||||
decode is replaced by a new message without wrapping the original error.
|
||||
Functional probes showed that an incomplete first document retained
|
||||
`io.ErrUnexpectedEOF`, while an incomplete trailing document lost it. A syntax
|
||||
error in the first document was available through `errors.As` as
|
||||
`*json.SyntaxError`; the corresponding trailing syntax error was not.
|
||||
|
||||
**Expected behavior:** Require one complete document, retain supported partial
|
||||
evidence/additional-field behavior, and preserve available parsing causes in
|
||||
the typed `record_format` error. Invalid extra data needs no materialized tree.
|
||||
|
||||
**Remediation opportunity:** Simplify loading to a complete-document unmarshal
|
||||
into the versioned envelope, retaining existing format/evidence validation and
|
||||
cause wrapping. An equivalent trailing-data check can also work if it avoids
|
||||
decoding an arbitrary second value and retains real parser failures. Add focused
|
||||
single-document, extra-document, trailing-whitespace, and malformed-suffix
|
||||
coverage. Keep raw record identity handling coordinated with R10; no new size
|
||||
caps or public input restrictions are needed.
|
||||
|
||||
## R13: Retain entries returned with a directory-enumeration error — P2
|
||||
|
||||
**Relevant code:** `internal/publish/attempts.go:41` through its immediate error
|
||||
return at lines 42–44; Stage 5's partial-result requirement in the implementation
|
||||
plan.
|
||||
|
||||
Go's `os.ReadDir` can return both entries read successfully and a later read
|
||||
error. Discovery checks the error first and immediately returns its still-empty
|
||||
result, dropping every returned entry. Thus a filesystem enumeration failure
|
||||
can hide readable attempts that were already discovered, even though the API
|
||||
and CLI support returning attempts alongside a top-level failure.
|
||||
|
||||
**Evidence:** A disposable probe supplied the directory-reader boundary with
|
||||
the real entry for one valid record plus a controlled enumeration error, an
|
||||
outcome permitted by `os.ReadDir`'s contract. `ListPublicationAttempts` returned
|
||||
zero attempts with `storage_enumeration`. Without the injected error, it
|
||||
returned that same record successfully. This was fault injection at the read
|
||||
boundary, not a change to the discovery algorithm under review.
|
||||
|
||||
**Expected behavior:** Preserve readable attempts and per-record diagnostics
|
||||
from returned entries while reporting the top-level enumeration failure. The
|
||||
operation remains incomplete and retains its non-success CLI classification.
|
||||
|
||||
**Complexity:** Processing a returned prefix of `d` entries containing `r`
|
||||
record candidates and `b` readable bytes takes the same ordinary filtering,
|
||||
ordering, and decoding work: `O(d + r log r + b)` with R11's refinement. The
|
||||
existing `os.ReadDir` approach instead retains its `O(d log d + b)` bound.
|
||||
Neither correction requires another directory scan: both retain the names
|
||||
already enumerated and perform the normal record loads for those names.
|
||||
|
||||
**Remediation:** Retain the enumeration error, process available entries using
|
||||
the normal discovery path, and return both the collected result and that error.
|
||||
Handle zero-entry failures and cancellation consistently with R04. Add a narrow
|
||||
fault-injected regression for entries-plus-error, verifying readable attempts,
|
||||
retained causes, and API/CLI partial-result behavior. This concerns data
|
||||
preservation, separate from R04's cancellation/error classification.
|
||||
|
||||
Reference in New Issue
Block a user