Skip to content

refactor(server): dispatch authored construction lineage through per-operation handlers - #206

Merged
matthewpeterkort merged 1 commit into
feature/postgres-backendfrom
ao/repo-simplify/193-authored-lineage-dispatch
Nov 13, 2026
Merged

matthewpeterkort merged 1 commit into
feature/postgres-backendfrom
ao/repo-simplify/193-authored-lineage-dispatch

Conversation

@matthewpeterkort

Copy link
Copy Markdown
Collaborator

Closes #193. Base: 80e8877 on feature/postgres-backend.

What

authoredConstructionOutputsWithCombineInputs (733 lines) walks authored construction steps, carries prior schema + authored lineage, maps each operation to outputs, mutates source metadata, and rejects compiler/authored identity mismatches. This PR extracts one private handler per operation kind (13 handlers) that receives an explicit constructionStepContext (document, step, stepIndex, prior schema, authored metadata, source-only projection IDs, compiled output, resolved Combine inputs, capability snapshot, seeded next metadata, shared addOutput validator) and returns the next metadata map or an error. The owner keeps the shared per-step schema transition: prior seeding with compiled source-stage merge, nextAuthored seeding, addOutput validation and error wrapping, step output-schema validation, clear(authored)/copy-back per-step atomicity, first-failure precedence, and the unsupported-kind default. Ownership stays in internal/server; no new package, no registry, no framework — one explicit switch, one static call per kind.

Why this reduces reasoning burden (not line count)

  • Before: answering "what does a UNPIVOT step require of authored metadata?" meant holding the whole 733-line function in mind and figuring out which shared lines matter for that operation. After: each operation's contract is one named function that fits on a screen, and its destructure prologue is a machine-checked manifest of exactly the shared state that operation reads.
  • Failure paths are addressable by name: grep applyUnpivotConstructionStep locates the melt/claim error paths; before, they were found by reading 400 lines of neighbours.
  • The shared validator cannot be bypassed or duplicated: handlers receive addOutput through the context; there is no operation-local re-implementation of declared-ID/name/label/duplicate/lineage checks, so the invariant is structural.
  • Future changes have smaller blast radius: touching the Group contract no longer requires re-deriving Combine or CodedPivot checks while reviewing; a reviewer reads one handler against one operation's wire shape.
  • The reviewable surface shrank: every moved line is proven line-exact by the tools below, so behavior review reduces to ~90 lines of glue (context struct, field literal, 13 dispatch lines, one error check).

Behavior-preservation evidence

Extraction is mechanical and rerunnable (>500 lines changed rule). Only two sanctioned rewrites: two-tab dedent, and single-value error returns gain a nil first value. Correspondence checker output (before/after moved-body, from the base blob):

applyGroupConstructionStep: source=126 moved=126 matched=126 destructured=5
applyCodedGroupConstructionStep: source=40 moved=40 matched=40 destructured=4
applyExpandConstructionStep: source=16 moved=16 matched=16 destructured=3
applyDeriveConstructionStep: source=73 moved=73 matched=73 destructured=7
applyPivotConstructionStep: source=37 moved=37 matched=37 destructured=6
applyCodedPivotConstructionStep: source=31 moved=31 matched=31 destructured=3
applyUnpivotConstructionStep: source=82 moved=82 matched=82 destructured=8
applyFilterConstructionStep: source=49 moved=49 matched=49 destructured=6
applyCombineConstructionStep: source=37 moved=37 matched=37 destructured=5
applyRelatedSourceConstructionStep: source=32 moved=32 matched=32 destructured=3
applyRelatedExpandConstructionStep: source=21 moved=21 matched=21 destructured=3
applyRelatedEligibilityConstructionStep: source=3 moved=3 matched=3 destructured=2
applyRelatedFieldConstructionStep: source=22 moved=22 matched=22 destructured=3
CORRESPONDENCE OK: 13 handlers, all moved bodies line-exact,
retained regions line-exact, error-literal multiset identical

The checker also proves: owner-retained regions (shared schema prelude, nextAuthored seeding, addOutput, output-schema validation + clear/copy-back, default kind error) are line-exact versus the base, and the function-region string-literal multiset is identical — exact public error texts and refusal messages preserved, nothing added or dropped. File prefix (lines 1-643) and suffix (combineProjectionInputName to EOF) are byte-identical versus base.

Checks (ticket checks[] + complete focused suites)

Check matched passed skipped failed
git diff --check — exit 0 — 0
go test -json ./internal/server -run 'TestAuthoredConstructionOutputs|TestDeriveSourceProjection|TestExplorerReceipt' -count=1 9 (3 top-level, 6 subtests) 9 0 0
issue regex Test(AuthoredConstructionOutputs|DeriveSourceProjection|AuthoredConstructionOutputsResolveCompilerSourceChildren|.*UnpivotSource|ExplorerReceipt) 28 28 0 0
issue regex TestExplorer.*(Receipt|Reconcile|Construction) 0 — — 0 (no tests to run, exit 0 at base and head — no test name matches this pattern)
go test ./internal/server -count=1 (full) 728 695 33 0
gofmt / go vet ./internal/server / go build ./... clean
scripts/check_no_code_comments.py on changed file scanned=1 violations=0

Full-suite equivalence is test-level: base 695 pass / 33 skip / 0 fail vs head identical, test-level diffs: NONE. The 33 skips are service-gated integration tests (Arango / ClickHouse / PostgreSQL fixtures), identical set at base and head, so those paths stay CI-gated rather than locally exercised. No test was modified; no new cases were added because per-test parity shows the extraction exposed no observable gap.

Scope honored

  • Only internal/server/explorer_output_reconcile.go touched (owned path). No frontend, no generated files, zero handwritten code comments (file had none; checker confirms).
  • No feature/behavior changes, no generic dispatch framework, no naming churn: the only new identifiers are the context struct and the 13 handler names. The 3-line authoredConstructionOutputs wrapper stays (it names the test-facing intent; removing it would add boilerplate to tests for no comprehension gain).
  • Existing source-integrity/ledger gates unaffected: verification/*/reviewed-sources.json contain no gated files rows for this path (prose round-records only), and no test reads this file's source.

Rerunning the transformation

python3 transform.py <repo> 80e88771309881e9d473d76dc2226ed10f204af9 <out> mapping.json
python3 verify_correspondence.py <repo> 80e88771309881e9d473d76dc2226ed10f204af9 <repo>/internal/server/explorer_output_reconcile.go mapping.json

transform.py reads the pristine base blob via git show, aborts on any anchor drift, and refuses on comment-count delta; the checker exits non-zero on any non-matching line.

transform.py
#!/usr/bin/env python3
"""Rerunnable extraction tool for repo-simplify-193.

Reads the pristine base blob of internal/server/explorer_output_reconcile.go
via `git show`, extracts each construction-operation case body verbatim into
one private handler that receives an explicit constructionStepContext, and
rewrites the file in place. Shared per-step machinery (schema validation,
nextAuthored seeding, addOutput, output-schema validation, clear/copy-back,
default unsupported-kind error, error wrapping) stays in the owner function.

Emits a JSON mapping (per handler: source line range, line counts) for the
correspondence checker. Aborts on any anchor drift.

Usage: transform.py <repo> <base-ref> <out-path> <mapping-json>
"""

import json
import re
import subprocess
import sys

REL = "internal/server/explorer_output_reconcile.go"

FN_START = "func authoredConstructionOutputsWithCombineInputs("
LOOP_START = "\tfor stepIndex, step := range document.Construction.Steps {"
ADDAOUT_START = ("\t\taddOutput := func(outputColumnID, constructionID string, "
                 "inputIDs []string, quality constructedOutputQuality) error {")
SWITCH_START = "\t\tswitch step.Operation.Kind {"
TAIL_START = "\t\tnext := make(map[string]string, len(step.Outputs))"
TAIL_END = "\t\tprior = next"

CASE_RE = re.compile(r"^\t\tcase authoringv2\.(\w+):$")
CTX_IDENTS = ["document", "step", "stepIndex", "prior", "authored",
              "sourceOnlyIDs", "compiledOutput", "resolvedInputs",
              "rowSourceSnapshot", "nextAuthored", "addOutput"]
STR_RE = re.compile(r"\"(?:\\\\.|[^\"\\\\])*\"|`[^`]*`")
RETURN_RE = re.compile(r"^return (err\b|fmt\.Errorf\()")

CTX_TYPE = """type constructionStepContext struct {
\tdocument          authoringv2.Document
\tstep              authoringv2.ConstructionStep
\tstepIndex         int
\tprior             map[string]string
\tauthored          map[string]authoredOutputColumn
\tsourceOnlyIDs     map[string]struct{}
\tcompiledOutput    lower.CompiledRecipeOutput
\tresolvedInputs    map[int]ir.ResolvedClickHouseTable
\trowSourceSnapshot capability.Snapshot
\tnextAuthored      map[string]authoredOutputColumn
\taddOutput         func(outputColumnID, constructionID string, inputIDs []string, quality constructedOutputQuality) error
}
"""

CTX_LITERAL = """\t\tstepContext := constructionStepContext{
\t\t\tdocument:          document,
\t\t\tstep:              step,
\t\t\tstepIndex:         stepIndex,
\t\t\tprior:             prior,
\t\t\tauthored:          authored,
\t\t\tsourceOnlyIDs:     sourceOnlyIDs,
\t\t\tcompiledOutput:    compiledOutput,
\t\t\tresolvedInputs:    resolvedInputs,
\t\t\trowSourceSnapshot: rowSourceSnapshot,
\t\t\tnextAuthored:      nextAuthored,
\t\t\taddOutput:         addOutput,
\t\t}
\t\tvar stepErr error
"""

DISPATCH = "\t\t\tnextAuthored, stepErr = apply{op}ConstructionStep(&stepContext)\n"

STEP_ERROR_CHECK = """\t\tif stepErr != nil {
\t\t\treturn stepErr
\t\t}
"""


def must_index(lines, needle, where):
    hits = [i for i, l in enumerate(lines) if l == needle]
    if len(hits) != 1:
        raise SystemExit(f"anchor drift at {where}: {needle!r} found {len(hits)} times")
    return hits[0]


def main():
    repo, base_ref, out_path, mapping_path = sys.argv[1:5]
    raw = subprocess.run(["git", "show", f"{base_ref}:{REL}"], cwd=repo,
                         capture_output=True, check=True).stdout.decode()
    lines = raw.split("\n")

    fn_start = must_index(lines, FN_START, "fn start")
    fn_end = next(i for i in range(fn_start, len(lines)) if lines[i] == "}")
    sig_open = must_index(lines, ") error {", "fn brace")
    assert fn_start < sig_open < fn_end
    loop = must_index(lines, LOOP_START, "loop")
    addout = must_index(lines, ADDAOUT_START, "addOutput")
    switch = must_index(lines, SWITCH_START, "switch")
    assert sig_open < loop < addout < switch < fn_end
    addout_end = next(i for i in range(addout + 1, len(lines)) if lines[i] == "\t\t}")
    assert addout < addout_end < switch

    cases = []
    switch_end = None
    default_body = None
    i = switch + 1
    while True:
        line = lines[i]
        if line == "\t\t}":
            switch_end = i
            break
        if line.startswith("\t\t\t") or not line.strip():
            i += 1
            continue
        m = CASE_RE.match(line)
        if m:
            cases.append((m.group(1), i))
        elif line == "\t\tdefault:":
            default_body = lines[i + 1]
            if not RETURN_RE.match(default_body.strip()) or \
               "unsupported operation kind" not in default_body:
                raise SystemExit("unexpected default case")
            i += 1
        else:
            raise SystemExit(f"unexpected switch line {i}: {line!r}")
        i += 1
    if len(cases) != 13 or default_body is None or switch_end is None:
        raise SystemExit(f"expected 13 cases + default, got {len(cases)}")

    bounds = []
    for idx, (op, cl) in enumerate(cases):
        body_start = cl + 1
        body_end = cases[idx + 1][1] if idx + 1 < len(cases) else next(
            j for j in range(cl + 1, switch_end) if lines[j] == "\t\tdefault:")
        bounds.append((op, cl, body_start, body_end))

    tail = must_index(lines, TAIL_START, "tail start")
    tail_end = must_index(lines, TAIL_END, "tail end")
    assert switch_end < tail < tail_end < fn_end

    mapping = {"base": base_ref, "file": REL, "function": {
        "name": "authoredConstructionOutputsWithCombineInputs",
        "source_lines": [fn_start + 1, fn_end + 1]}, "handlers": []}

    def used_idents(body):
        text = STR_RE.sub('""', "\n".join(body))
        return [ident for ident in CTX_IDENTS
                if re.search(r"\b" + ident + r"\b", text)]

    handlers = []
    for op, cl, bs, be in bounds:
        body = lines[bs:be]
        out_body = []
        for l in body:
            s = l.strip()
            if not s:
                continue
            if s.startswith("return "):
                if not RETURN_RE.match(s):
                    raise SystemExit(f"unhandled return in {op}: {s!r}")
                s = "return nil, " + s[len("return "):]
            leading = len(l) - len(l.lstrip("\t"))
            out_body.append("\t" * max(leading - 2, 1) + s)
        idents = used_idents(body)
        if "nextAuthored" not in idents:
            idents.append("nextAuthored")
        op_short = re.sub(r"^ConstructionOperation", "", op)
        fname = f"apply{op_short}ConstructionStep"
        h = [f"func {fname}(stepContext *constructionStepContext) "
             f"(map[string]authoredOutputColumn, error) {{"]
        for ident in CTX_IDENTS:
            if ident in idents:
                h.append(f"\t{ident} := stepContext.{ident}")
        h.extend(out_body)
        h.append("\treturn nextAuthored, nil")
        h.append("}")
        handlers.append("\n".join(h))
        mapping["handlers"].append({
            "handler": fname, "operation": op,
            "source_lines": [cl + 1, be],
            "source_body_lines": be - bs,
            "moved_body_lines": len(out_body),
            "destructured_idents": idents})

    prelude = lines[sig_open + 1:loop]
    seed = lines[loop:addout]
    addout_block = lines[addout:addout_end + 1]
    tail_block = lines[tail:tail_end + 1]
    loop_close = lines[tail_end + 1:fn_end + 1]

    new_fn = [FN_START]  # signature lines kept from original
    new_fn = lines[fn_start:sig_open + 1] + prelude + seed + addout_block
    new_fn.append(CTX_LITERAL.rstrip("\n"))
    new_fn.append(SWITCH_START)
    for op, _, _, _ in bounds:
        new_fn.append(f"\t\tcase authoringv2.{op}:")
        new_fn.append(DISPATCH.format(
            op=re.sub(r"^ConstructionOperation", "", op)).rstrip("\n"))
    new_fn.append("\t\tdefault:")
    new_fn.append(default_body)
    new_fn.append("\t\t}")
    new_fn.extend(STEP_ERROR_CHECK.rstrip("\n").split("\n"))
    new_fn.extend(tail_block)
    new_fn.extend(loop_close)

    out = lines[:fn_start] + CTX_TYPE.split("\n") + new_fn + [""]
    for h in handlers:
        out.extend(h.split("\n"))
        out.append("")
    out.extend(lines[fn_end + 1:])

    text = "\n".join(out)
    if text.count("//") != raw.count("//"):
        raise SystemExit("comment delta introduced")
    with open(out_path, "w") as f:
        f.write(text)
    with open(mapping_path, "w") as f:
        json.dump(mapping, f, indent=1)
    print(f"wrote {out_path}: {len(lines)} -> {len(out)} lines; "
          f"{len(mapping['handlers'])} handlers")


if __name__ == "__main__":
    main()
verify_correspondence.py
#!/usr/bin/env python3
"""Before/after moved-body correspondence checker for repo-simplify-193.

Proves, from the pristine base blob and the rewritten file:
  1. every handler body line equals its source case body line, modulo the
     only two sanctioned rewrites (2-tab dedent; single-value error return
     gains a nil first value), in order, with zero missing/unexpected lines;
  2. the owner-retained regions (shared prelude, seed, addOutput, output
     schema validation, clear/copy-back, default kind error) are identical
     modulo indentation;
  3. the multiset of string literals in the function region is byte-identical
     (exact public error text preserved; nothing added or dropped);
  4. the function region contains no comments and the file's comment count
     is unchanged.

Usage: verify_correspondence.py <repo> <base-ref> <new-file> <mapping-json>
Exits non-zero on any violation.
"""

import json
import re
import subprocess
import sys

REL = "internal/server/explorer_output_reconcile.go"
RETURN_PREFIXES = ("return nil, err", "return nil, fmt.Errorf(")


def strip_lines(text):
    return [l.strip() for l in text.split("\n") if l.strip()]


def region(lines, start_marker, end_marker):
    s = lines.index(start_marker)
    e = lines.index(end_marker, s)
    return lines[s:e + 1]


def rewrite_source_body(body):
    out = []
    for l in body:
        s = l.strip()
        if not s:
            continue
        if s.startswith("return ") and not s.startswith("return nil"):
            s = "return nil, " + s[len("return "):]
        out.append(s)
    return out


def extract_func(text, name):
    start = text.index(f"func {name}(")
    end = text.index("\n}\n", start) + 3
    return text[start:end]


def main():
    repo, base_ref, new_path, mapping_path = sys.argv[1:5]
    old = subprocess.run(["git", "show", f"{base_ref}:{REL}"], cwd=repo,
                         capture_output=True, check=True).stdout.decode()
    new = open(new_path).read()
    mapping = json.load(open(mapping_path))
    old_lines = old.split("\n")
    failures = []
    report = []

    old_fn = extract_func(old, "authoredConstructionOutputsWithCombineInputs")
    new_fn = extract_func(new, "authoredConstructionOutputsWithCombineInputs")

    def case_body(op):
        marker = f"\t\tcase authoringv2.{op}:"
        s = old_fn.index(marker) + len(marker) + 1
        rest = old_fn[s:]
        ends = [rest.index(m) for m in ("\n\t\tcase ", "\n\t\tdefault:") if m in rest]
        return old_fn[s:s + min(ends)].split("\n")

    for h in mapping["handlers"]:
        fname, op = h["handler"], h["operation"]
        expected = rewrite_source_body(case_body(op))
        hf = extract_func(new, fname)
        body = hf.split("\n")[1:-2]
        gen_end = body.index("\treturn nextAuthored, nil")
        pre = [l for l in body[:gen_end] if re.match(
            r"^\t\w+\s*:=\s*stepContext\.\w+$", l)]
        moved = [l.strip() for l in body[len(pre):gen_end]]
        for l in pre:
            if not re.match(r"^\t\w+\s*:=\s*stepContext\.\w+$", l):
                failures.append(f"{fname}: bad destructure {l!r}")
        if moved != expected:
            for i, (a, b) in enumerate(zip(expected, moved)):
                if a != b:
                    failures.append(f"{fname}: line {i}: source={a!r} new={b!r}")
                    break
            if len(expected) != len(moved):
                failures.append(f"{fname}: count {len(expected)} != {len(moved)}")
        for l in moved:
            if l.startswith("return ") and not l.startswith(RETURN_PREFIXES):
                failures.append(f"{fname}: uncanonicalized return {l!r}")
        report.append(f"{fname}: source={h['source_body_lines']} "
                      f"moved={len(moved)} matched={sum(1 for a, b in zip(expected, moved) if a == b)} "
                      f"destructured={len(pre)}")

    retained = [
        ("shared schema prelude",
         old_fn[old_fn.index("\tif document.Construction == nil {"):
                old_fn.index("\tfor stepIndex, step := range")],
         new_fn[new_fn.index("\tif document.Construction == nil {"):
                new_fn.index("\tfor stepIndex, step := range")]),
        ("nextAuthored seeding",
         old_fn[old_fn.index("\tnextAuthored := make("):
                old_fn.index("\t\taddOutput := func(")],
         new_fn[new_fn.index("\tnextAuthored := make("):
                new_fn.index("\t\taddOutput := func(")]),
        ("addOutput validation",
         old_fn[old_fn.index("\t\taddOutput := func("):
                old_fn.index("\t\tswitch step.Operation.Kind {")],
         new_fn[new_fn.index("\t\taddOutput := func("):
                new_fn.index("\t\tstepContext := constructionStepContext{")]),
        ("output schema + clear/copy-back",
         old_fn[old_fn.index("\t\tnext := make(map[string]string, len(step.Outputs))"):
                old_fn.index("\treturn nil\n}")],
         new_fn[new_fn.index("\t\tnext := make(map[string]string, len(step.Outputs))"):
                new_fn.index("\treturn nil\n}")]),
    ]
    for name, a, b in retained:
        if strip_lines(a) != strip_lines(b):
            failures.append(f"retained region differs: {name}")

    old_switch = region(old_fn.split("\n"), "\t\tdefault:", "\t\t}")[1]
    new_switch = region(new_fn.split("\n"), "\t\tdefault:", "\t\t}")[1]
    if old_switch.strip() != new_switch.strip():
        failures.append("default unsupported-kind error differs")

    str_re = re.compile(r"\"(?:\\\\.|[^\"\\\\])*\"")
    old_all = old_fn
    new_all = new_fn + "".join(extract_func(new, h["handler"])
                               for h in mapping["handlers"])
    if sorted(str_re.findall(old_all)) != sorted(str_re.findall(new_all)):
        failures.append("string literal multiset changed in function region")
    for h in mapping["handlers"]:
        if extract_func(new, h["handler"]).count("//"):
            failures.append(f"{h['handler']}: comment introduced")

    for line in report:
        print(line)
    if failures:
        for f in failures:
            print("FAIL:", f)
        raise SystemExit(1)
    print("CORRESPONDENCE OK: %d handlers, all moved bodies line-exact, "
          "retained regions line-exact, error-literal multiset identical"
          % len(mapping["handlers"]))


if __name__ == "__main__":
    main()

…operation handlers

Extract one private handler per construction operation kind from
authoredConstructionOutputsWithCombineInputs (733 lines) into named
apply<Operation>ConstructionStep functions that receive an explicit
constructionStepContext (document, step, stepIndex, prior schema, authored
metadata, source-only projection IDs, compiled output, resolved Combine
inputs, capability snapshot, seeded next metadata, and the shared addOutput
validator) and return the next metadata map or an error. The owner keeps
the shared per-step schema transition: source-projection exclusion set,
prior/priorIDsByName seeding with compiled source-stage merge, nextAuthored
seeding, addOutput validation and error wrapping, step output-schema
validation, clear(authored)/copy-back per-step atomicity, first-failure
precedence, and the unsupported-kind default error.

Behavior is preserved by construction, not by assertion tuning: 569
case-body lines moved line-exact under the only two sanctioned rewrites
(two-tab dedent; single-value error returns gain a nil first value), all
owner-retained regions line-exact, and the function-region string-literal
multiset identical, verified against the base blob by a rerunnable
transformation plus a correspondence checker.

Verification: go test ./internal/server -count=1 gives the identical
test-level result as the base (695 pass, 33 service-gated skips, 0 fail);
focused runs TestAuthoredConstructionOutputs|TestDeriveSourceProjection|
TestExplorerReceipt (9 pass) and the operation-contract regex (28 pass)
match base; TestExplorer.*(Receipt|Reconcile|Construction) matches zero
tests at base and after; gofmt, go vet, scripts/check_no_code_comments.py
and git diff --check clean.

Issue: #193
@matthewpeterkort

Copy link
Copy Markdown
Collaborator Author

CI recovery note

First run failed Tests / Node unit tests on browser tree does not outlive a deep failed run (post-launch throw) (scripts/verify-ui/helpers/tests/row96-harness-lifecycle.test.mjs:154).

@matthewpeterkort matthewpeterkort left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: PR #206 @ 8a325a2 — per-operation construction lineage handlers

Verdict: APPROVED — no changes requested; ready. (Posted as COMMENT because GitHub rejects APPROVE/REQUEST_CHANGES on own-account reviews; this sentence carries the verdict.)

Scope reviewed. Full diff against base 80e8877 on feature/postgres-backend: one file, internal/server/explorer_output_reconcile.go (+710/-556), two hunks. I compared the base function body (old lines 644-1376) against the new driver and all 13 apply*ConstructionStep handlers line by line.

What I verified:

  • Faithful extraction: every handler body matches the corresponding base case — error strings and %w wrapping (derive/pivot/coded-pivot/unpivot quality errors), addOutput call order, break/continue flow, group key/aggregate/row-value loops, source-projection claim gates for derive/unpivot/filter, combine projection resolution, and related-* metadata writes are all behaviorally identical.
  • Owner keeps the shared per-step transition unchanged: prior seeding with the compiled source-stage merge, per-step nextAuthored seeding, addOutput validation, step output-schema validation, clear(authored)/copy-back atomicity, first-failure precedence, and the unsupported-kind default.
  • Error semantics preserved: handlers return (nil, err) and the driver returns stepErr before clear(authored)/prior advance, so caller-visible authored/prior maps see no partial mutation on failure, same as the old direct returns.
  • Shared-state aliasing is consistent: stepContext.nextAuthored, handler-local copies, and the addOutput closure all reference the single per-step map, and every handler returns that same map.
  • Wiring: 13 handler definitions, 13 switch dispatch sites, one-to-one; default branch identical to base.
  • CI on 8a325a2: all 15 checks green including Go unit tests; existing suites (unpivot/derive source projection, source-child, receipt-contract, proposal) drive these paths through the entry points and assert extracted error strings such as the snapshot-claim message.

Optional suggestions (no action required):

  • nextAuthored now travels through two channels: the constructionStepContext field and the handler return value. All handlers mutate-and-return the same map today, so behavior is identical; but if a future handler returned a fresh map, addOutput writes and its duplicate check (via the captured loop variable) would target the seed map and be silently dropped from the copy-back — a latent validation-bypass footgun. Consider single-channel state (context field only) or keeping the return but documenting/enforcing the same-map invariant.
  • Direct table-driven tests on each apply* handler would pin the per-operation failure paths the PR advertises (grep-by-name debuggability) at handler granularity, complementing the entry-point coverage.

@matthewpeterkort
matthewpeterkort marked this pull request as ready for review November 13, 2026 21:17
@matthewpeterkort
matthewpeterkort merged commit 8943086 into feature/postgres-backend Nov 13, 2026
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant