mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
xarmian
21b4d8d9c4
* test(store): measure the item-side rename cascade amplification (BUG-2804) Measurement only — no production code changes, no cap. The dispatch is measure-first, and BUG-2798's arithmetic is explicitly non-transferable to this path, so nothing here reuses its numbers. internal/store/items_rename_probe_test.go sweeps three axes with a k=0 negative control and a precondition that fails if the cascade would not be exercised (an empty cascade would otherwise report a flat line and read as "no amplification"). Instrument is MemStats.TotalAlloc, the same counter documents_rename_bounds_test.go uses, so the two paths compare directly. What it establishes, all measured on this machine: - 2.01x the body allocated per bracket, linear across four intervals (1 -> 8 -> 64 -> 512 -> 4096 brackets, agreeing to three significant figures). Bracket count is not independent of body size: the cheapest link is 5 bytes and nothing caps links per item, so the cost is O(C^2) in ONE linker's body, in ONE request, with no accumulation needed. Confirmed directly: doubling C multiplies allocation by 3.83 / 3.92 / 3.95, converging on 4. - 21.3x-22.2x the body per added linker at 8 brackets each, linear in k. - The outbox payload carries every rewritten body in a single row, at exactly 1.00x the body set, persisted. emitBulkItemEventTx documents this as deliberately unbounded in v1 and invites the measurement. Peak residency is reported as a coarse floor and carries no claim. The sampler sleeps between reads deliberately: ReadMemStats stops the world, and a spin loop turned a sub-second cascade into minutes of wall clock on this probe's first run. * test(store,links): decompose the per-bracket constant into its two sites (BUG-2804) Still measurement only. Checkpoint 2 reported a marginal cost of 2.01x the body per bracket without establishing what the 2 was made of. These probes account for it exactly, and the answer changes the fix shape. - internal/links/probe_alloc_test.go isolates RewriteBracketAt: 1.00x the body per call. So the rewriter is only HALF the per-bracket cost. - items_rename_probe_test.go's SELECT probe finds the other half, and it is not in the rewriter at all. cascadeTitleRename's SELECT joins item_wiki_links to items and projects s.content, returning one row per LINK, each carrying a full copy of the source body. The scan loop de-duplicates by source id only AFTER rows.Scan has allocated a fresh string per row. Measured dead flat at 1.00x the body per row against a SINGLE distinct source: 4096 rows on one 256 KiB item allocate 1.07 GB. 1.00 + 1.00 accounts for the measured 2.01. That makes TWO independent quadratics with different fixes. A single-pass rewriter would leave the SELECT's copy untouched and halve the cost rather than remove it. A third probe tests, and REFUTES, an earlier hypothesis that the second copy was the cascade's `rewritten != newContent` full-body comparison: a different-length new title measures 2.05x against the same-length 2.02x, no drop. The comparison allocates nothing, so it could never have appeared in an allocation counter. Kept as a negative result so the refuted reading is not re-derived. * refactor(links): single-pass RewriteBracketsAt, RewriteBracketAt delegates (BUG-2804 M2) The per-bracket decision moves into one unexported helper; RewriteBracketAt becomes the one-element case so the two cannot drift. Differential tests pin behaviour against the PRE-REFACTOR implementation, frozen verbatim in the test file as an oracle. Comparing RewriteBracketAt to RewriteBracketsAt-of-one would be vacuous now that the former is defined as the latter, so the oracle is what makes the test able to fail. * fix(store): bound the item rename cascade and remove both quadratics (BUG-2804) Three mechanisms, landing together because the first two are factors of one product — either alone still leaves O(C^2) while measuring "2x better". M1: cascadeTitleRename's SELECT no longer projects s.content. It returns one row per LINK, so projecting the body made the driver materialise a full copy per bracket, de-duplicated only after rows.Scan had allocated each one. Measured at 1.00x the body per row against a SINGLE source. Each source's content is now read once. M2: the rewrite is a single pass. links.RewriteBracketsAt splices every recorded bracket with one strings.Builder; RewriteBracketAt is reimplemented as the one-element case so the per-bracket decision has exactly one copy and the public contract is unchanged. M3: MaxItemRenameCascadeBytes (64 MiB) bounds the total linking-item content one rename may process, charged before each body is built so a refusal never allocates the thing it refuses. The number is derived from the live workspace's measured distribution, not inherited from documents.go, and the receipt is in the constant's doc comment. Measured, before -> after: doubling C 3.83/3.92/3.95x -> 2.04/2.21/2.06x (quadratic gone) per bracket 2.01x body -> 0.01x body per linker 21.3x body -> 7.3x body 4096-row SELECT 1,074,850,200 B -> 1,180,856 B (910x) store suite 180.9s -> 86.8s Equivalence is pinned against the PRE-REFACTOR implementation, frozen in the test file as an oracle, over a hand-enumerated behaviour corpus plus 20,000 randomised inputs; and the single pass is pinned against the descending fold the cascade used to perform, over 5,000 randomised inputs with shuffled, duplicated and corrupted offsets. The k-linear retention that remains is in the outbox member snapshots, filed separately as BUG-2827. * fix(store,server): codex R1 findings + a dialect-dependent test instrument (BUG-2804) P1 — ProjectRewrittenLen built a replacement string per bracket to measure it, reintroducing allocate-then-refuse one layer below the cap it feeds. The per-bracket helper now returns the SEGMENTS and never concatenates; projection does length arithmetic only, and the rewrite writes the parts straight into its builder, removing an allocation from the hot path too. P2 — the item rename cascade's refusal had no errors.As arm in handleUpdateItem, so a deliberate, permanent decline reached clients as a 500 implying a retry might help. Now 413 rename_cascade_too_large, composed from the error's typed fields so the internal call path is not published. Pinned by an exact-status test plus a counterfactual that a blanket-413 handler would fail. Checked the sibling write paths rather than assuming the reported one was the population: bulk update and version restore never set Title, so the cascade cannot fire there and this handler is the whole surface. P2 — `applied` counted brackets MATCHED, not brackets CHANGED, so a rewrite reproducing content byte-for-byte would still write the row, bump seq and emit events. Counting changes restores what the pre-fix whole-body comparison did. Cascade reachability is NOT established and the test says so. Separately, found by make test-pg: the refuse-before-building guard used a TotalAlloc ceiling, and the same refusal allocates ~69 MB on SQLite against ~242 MB on Postgres — the ceiling was measuring the driver. Replaced with an exact count of bodies built via a store test seam. The first version of that seam ALSO failed to discriminate: a mutation moving the cap check between the build and the seam call went undetected, so the build and the count are now one function with no third position for the defect to hide in. Gates on this tree: gofmt, go vet, go build ./... clean; go test ./... PASS on SQLite AND on Postgres (own container, private port 5446 — never the shared 5445, whose teardown would kill a sibling seat's container mid-run). * fix(links,server): codex R2 findings (BUG-2804) R2-1 — ProjectRewrittenLen advanced its cursor past a no-op bracket while RewriteBracketsAt did not, so the two disagreed about which overlapping rewrites the guard skips. On `[[A[[B]]]]` with a no-op at 0 and a change at 3, projection reported 10 bytes / 0 applied while the pass produced 13 / 1 — the cascade charged the read and never charged the rewrite it then performed, so the bound leaked on exactly the corrupt and duplicated offsets the defensive paths exist for. Projection now follows the pass, not the reverse: the pass's behaviour is pinned to the descending fold the cascade used to perform, so moving it would have changed cascade semantics under cover of a bug fix. The lockstep property test that pins this needed MIXED per-position target titles — the first version shared one title across a call and passed against the broken code, because the reproducing shape needs a no-op bracket overlapping a changing one, which a shared title cannot express. Cascade rows carry per-row target_title, so mixed is the realistic case. R2-2 — the 413 mapping covered one of THREE places handleUpdateItem reaches UpdateItemWithParentLink. The post-R1 population sweep asked whether other HANDLERS reach the store call and never asked whether this handler reaches it more than once. All three now share writeItemRenameCascadeTooLarge. The collab-edit half had a deeper cause than a missing arm. In applyContentViaCollabOnce's prune-and-direct-write fallback, a failing directWrite() had its error DISCARDED and the original collab error returned in its place, so the caller read a deterministic refusal as a recoverable routing problem and fell through to its own direct write — re-deriving the identical refusal from scratch. Measured 64 rewritten bodies built for one request against an expected 32. Deterministic failures now survive that branch, scoped to the three errors every call site already treats as final (open-children rejection, update conflict, cascade refusal). Everything else still returns the collab error, preserving the graceful-degradation contract the branch exists for. Status alone cannot discriminate the double-work fix — both behaviours end in 413, since the fall-through reaches the plain path's arm — so the store's build observer is now reachable from the server package via an exported test-support setter, and the test asserts the work count. Gates: gofmt, vet, go build ./... clean; go test ./... PASS on SQLite and Postgres; go test -race PASS on internal/links and internal/store with zero data races. Note -race on store runs 705s, past the 600s default timeout. * test(links): make the multi-bracket fold oracle independent (BUG-2804, codex R3) The descending-fold equivalence test built its oracle by folding RewriteBracketAt — which now delegates to RewriteBracketsAt, the function under test. The comparison was therefore circular: it could agree with itself while both diverged from the pre-refactor behaviour it exists to pin. This is the same vacuity I flagged and avoided for the ONE-element assertion, missed one case over in the multi-bracket one. The fold now uses rewriteBracketAtV0, the already-frozen pre-refactor primitive, which is the only genuinely independent oracle available. The test still passes, so the single pass does match true pre-refactor behaviour on every generated input including the overlapping ones — that agreement is now evidence rather than a tautology. Mutation-verified after the change: the fold test kills the dropped-sort and dropped-overlap-guard mutants, two of which it had previously left to other tests. Test-only. `git diff8ddfdfd6-- . ':!*_test.go'` is empty, so the Postgres and store -race results recorded for8ddfdfd6carry over unchanged. * fix(store): bound the cascade SCAN, not just the rewrite loop (BUG-2804, codex R4) `works` holds one entry per matching LINK ROW, each carrying that row's target_title, and item titles have no length bound. So the scan retained rows x title bytes before the per-source loop fired, and the content-bytes cap could not see it. The doc comment on MaxItemRenameCascadeBytes claimed the opposite — that the cascade's retention was O(1) in the linker count, with the k-linear remainder attributed to the outbox (BUG-2827). Both halves were wrong: the outbox is a different vector, and this function had k-linear retention of its own. That comment was mine, and it is the claim this unit is named after. The scan is now charged against the same budget and refuses DURING the scan, so at the moment of refusal the process holds only rows already counted. One budget covers both phases deliberately: index rows and content bytes are different quantities, but both are memory this one rename makes the server hold, and a second constant would be a second thing to tune with no separate meaning. Measured: refusal fires at 67,113,728 bytes against the 67,108,864 cap — 4,864 past crossing — with zero bodies built, on a fixture carrying 64 rows of 2 MiB titles whose full scan would have reached ~134 MB. Three test corrections, all mine: - The attack I first designed is impossible. A link to a T-byte title costs T bytes of body text, so content and retained-title bytes are COUPLED and content is always larger. The fixture guard caught it. The property that separates the two halves is WHEN the refusal fires, not size. - Two tests pinned an exact admitted count, which the new scan charge shifts by a source; one of them also used a body size that divided the cap evenly, so refuse-after-build totalled exactly the cap and could not have been detected. Both now assert that the work actually done fits the budget. - The scan test is SQLite-scoped with a measured reason: on Postgres the huge-title fixture cannot be built at all, because items carries UNIQUE(workspace_id, slug), the slug is derived from the title without truncation, and the btree tuple limit is 8191 bytes. Filed separately as BUG-2831. The bound itself is NOT dialect-specific — row count is unbounded on both backends. Gates: gofmt, vet, go build ./... clean; go test ./... PASS on SQLite and Postgres; go test -race PASS on internal/links and internal/store; zero failures, zero data races. * perf(links): never build the replacement in the projection path (BUG-2804, codex R5) bracketRewriteAt composed `collSlug + "/" + newTitle` BEFORE the match check, so every bracket paid for it including the leave-alone exit — and ProjectRewrittenLen calls it once per rewrite BEFORE the cascade's cap can fire. R1's shape, one layer down. It does not document away. BUG-2831 established that item titles carry no validation bound, so a multi-megabyte newTitle is admissible and a bracket-dense body projected gigabytes of immediately-discarded allocation ahead of the refusal that exists to prevent exactly that. The helper now MATCHES FIRST and returns a DESCRIPTION — a `qualified` flag for the slug prefix plus a display-suffix slice of the body — so nothing proportional to newTitle is built anywhere in the projection path. The real pass writes "[[", collSlug, "/", newTitle, suffix, "]]" as separate segments. bracketUnchanged compares segment-wise rather than concatenating a candidate, which would have reintroduced the same allocation to answer a comparison. The `ttLower + "|"` concatenation is gone too, spelled out as the same predicate on the same two already-lowered strings. Measured, 4096 brackets against a 1 MiB newTitle: pre-R5 shape 4,328,782,464 bytes 1.0079x newTitle per bracket now 196,704 bytes 0.0000x The second row is a new probe; the first is that probe run against a mutant carrying the old shape, not arithmetic. No semantic change, and the frozen pre-refactor oracle is what establishes that: the full differential suite passes, including 20,000 randomised inputs and the projection/pass lockstep property. The slice-growth transient stays as documented. Gates: gofmt, vet, go build ./... clean; go test ./... PASS on SQLite and Postgres; go test -race PASS on internal/links and internal/store; zero failures, zero data races.