Repository navigation
Hygiene sweep: reuse, simplification and tooling-performance leftovers (R1-R3, R5, R6, S1-S8, P6, P7) #24
Description
Source: multi-agent review of PR #16; all R/S items UNVERIFIED-FINDER (maintenance/drift observations), P6/P7 CONFIRMED tooling-only. 2026-08-13. M-anchor: swept opportunistically whenever a listed file is touched (most live in binder/emitters M2 reworks anyway); anything still open at M6 operational closure is resolved there — nothing on this list survives the M series.
Reuse / drift:
- R1 compile probe re-types the provenance header (
tests/OpenCode.Sdk.Tools.Tests/Support/GeneratedSourceCompiler.cs:50-53) instead ofGenerationProvenance.HasHeader→ wording change causes false duplicate-type reds. Use the shared helper (or a shared byte/span helper). - R2
PipelineTestsreimplementsUserAgentPolicy.Compose(PipelineTests.cs:307-315) — derive expectations through the policy; keep one metadata-integration test. - R3 Basic-auth tests rebuild the header with hard-coded
opencode(PipelineTests.cs:52,69) vsPipeline.BasicUser— centralize the expected wire credential in a test contract constant/builder; do not expose production privates solely for tests. - R5 nullable readonly field emission duplicated (
ClientEmitter.cs:99-105,EnvelopeEmitter.cs:53-60) →EmissionSyntaxif both stay structurally identical. - R6
SchemaNameResolverreparsesGraphKeyBuilder.UnionBranch'smarker=valueformat ad hoc with silent whole-token fallback (SchemaNameResolver.cs:102-113) → centralize branch-key format/parse; refuse malformed identities.
Simplification (maintainer picks scope; none delays correctness):
- S1 one-field
PendingOperationPlan→IReadOnlyList<string>· S2 primary-ctor mirror fields (OperationPlanBinder.cs:200-207) · S3RefuseNull/RefuseNullTags→ one generic (:529-539) · S4 unusedclientparam inEmitPipelineAssignments(ClientEmitter.cs:194-204) · S5 unusedclientinEmitHandleFactory(:283-307) · S6 duplicated ctor guard/assignment syntax (:181-230) · S7 storedIsHandleOperationderivable fromPlan.Parameters(OperationPlanBinder.cs:186-197,237) · S8 constantClientPlan.Namespace(ClientPlan.cs:7).
Tooling perf (speed matters in tooling — standing posture; fix when in the file):
- P6
GenerationWritertriple-reads every owned file + whole-fileHasHeadersplit + repeatedResolveOwnedPathnormalization (GenerationWriter.cs:40,48,57,250) → fold provenance into the before-snapshot read, span header compare, cache validated paths per run. - P7
WriteSourcesAsynccopies everyReadOnlyMemory<byte>viaToArray()(:149) before the formatter rewrites files → stream/memory write seam onIFileSystem; only skip formatting with proof of canonical bytes.
Activity
Blind-Striker commentedon Aug 14, 2026
PR #26 verification (2026-08-14) adds seven low-severity hygiene items to this sweep — verifier detail in the transient
.scratchpad/ultra-review-m2/verify/RESULTS.json(ids below), durable record here:QueryRequestPlan.DerivesFromListRequestand per-propertyIsInheritedencode one fact twice; a mismatch silently mis-emits. Collapse to a single source of truth (or a construction-time invariant).generate --verifyrereads and rewrites the complete generated set (provenance pass + two snapshots + format over unchanged bytes). Tooling-perf only.SchemaAliasApplierreconstructs the full projected document for one sparse alias. Tooling-perf only.SessionResponseEnvelope≡SessionCreateResponseEnvelope); shape-keyed dedup would shrink the registry.UnionNormalizer's oneOf purity wall duplicates the anyOf predicate verbatim — unify so a future constraint cannot fork the mirrored rules.ResponseAdapterEmitter.CreateSuccessduplicates the common status/payload initializer for the cursor arm — unify.BoundOperation.Groupis dead state since grouping moved to the curated client name — delete.docs(agents): hand off the M2 close-out — Q91 research, fix batch, me…
Blind-Striker commentedon Aug 17, 2026
The
1ee8c30review adds confirmed correctness riders to the tooling areas already owned here.GenerationWriter safety and recovery
RefuseSymbolicLinkschecks children but notOutputRootitself, so a symlinked root can redirect writes outside the intended physical tree.Models/widget.csdoes not block plannedModels/Widget.cs; exactFile.Existsmisses the case-insensitive ownership collision and both files remain.Add real-filesystem tests where the fake cannot model case/symlink behavior. Correctness must land before P6/P7 writer optimization.
Small correctness/hygiene riders
SpecDocument.Schemasretains a caller-owned dictionary despite the immutable SpecIR contract; take an ordinal defensive copy.Trigger: next
GenerationWriter/tool-error handling change; no later than M6 operational closure.Blind-Striker commentedon Aug 18, 2026
Hosted CI evidence for a new tooling-test determinism rider: run 32128587050 attempt 1 failed only on Windows in OperationPlanBinderTests.Bind_Should_Refuse_A_Payload_Name_Colliding_With_Record_Members(PrintMembers). Microsoft.OpenApi ingestion translated a RegexMatchTimeoutException at the document boundary; the same test and full Windows matrix passed unchanged on attempt 2 at the identical SHA 7123627, while Linux/macOS were green. This was unrelated to the alias increment and was not hidden or retried locally. If it recurs, investigate reader regex timeout behavior under Windows/parallel test load and add deterministic containment rather than a retry/skip. Evidence: https://github.com/Blind-Striker/opencode-sdk-dotnet/actions/runs/32128587050
Blind-Striker commentedon Sep 14, 2026
Status 2026-09-14: still an opportunistic M6 sweep; nothing here blocks the preview line. R1–R3 and S-items remain drift observations to take when the file is next open; P6/P7 remain tooling-perf leftovers in GenerationWriter.
[-]M6 (opportunistic earlier): hygiene sweep — reuse/simplification/tooling-perf leftovers (R1-R3 R5 R6, S1-S8, P6 P7)[/-][+]Hygiene sweep: reuse, simplification and tooling-performance leftovers (R1-R3, R5, R6, S1-S8, P6, P7)[/+]Blind-Striker commentedon Oct 14, 2026
Milestone 1.1; the retired M-series anchor in the body no longer applies. No row of this issue moved to a 1.0 issue. The generator performance baseline (#186) measures the P6, P7 and V31 candidates before any of them is optimized. The remaining rows are trigger-based: each is taken when its file is next changed, and the
GenerationWritercorrectness riders still land before P6 and P7.