[ajitpratap0]

Summary

#618 linter: L006 and text-based L009 results depend on map iteration order

Fixes #618. Two linter rules produced different results on identical runs because they read their results from Go maps, whose iteration order is random. In L006 (column alignment), the expected indent is now chosen by scanning the continuation-column indents in source order, so when indent counts tie the earliest one wins. 'SELECT a,\n b,\n c\nFROM t' now reports line 3, column 5 on every run (100 of 100 CLI runs). The text-fallback path of L009 (aliasing consistency) now lists violations in the order tables first appear in the query; counts, messages and line numbers are unchanged. The new repeated-run tests in the style package and in gosqlx.Lint fail on 61ca6c8 in 5 of 5 runs and pass with -count=5 -race after the fix. The full race gate, go vet and golangci-lint are clean. Deferred, with sound reasons: (a) sorting all violations by position in LintString — optional in the issue, and it would change the existing per-rule grouping; (b) the cfg.Rules map ranged over in pkg/linter/config/config.go, which gives cfg.Warnings a random order — the same kind of bug but not reported in #618, so it should get a follow-up issue; (c) #619 is handled in its own commit on this chain.

#619 gosqlx: ExtractMetadata and Extract* return tables/columns/functions in random map order

ExtractTables, ExtractTablesQualified, ExtractColumns, ExtractColumnsQualified, ExtractFunctions and ExtractMetadata now return their results in sorted order, so repeated calls on the same AST return identical slices (#619). Before, the deduplicated keys came out in Go's random map-iteration order. Strings are now sorted in ascending byte order. QualifiedName results are sorted by Schema, then Table, then Name. The sort runs before the existing detach/arena copy on every path, so input-retention protection and allocs/op are unchanged; ns/op rises about 3-15% on the Extract benchmarks. New regression tests pin the exact output for the issue SQL and other queries over 50 calls, and check that every slice is sorted and that ExtractMetadata matches the individual extractors. The tests fail on the pre-fix code and pass with the fix. Out of scope: first-appearance (source) ordering, rejected in favour of sorting, which the issue allows; case-insensitive ordering; and docs/CHANGELOG updates, which are handled separately.

Commits (2), one implementation commit per issue plus test-only follow-ups:

• 609fa8d fix(gosqlx): return Extract* and ExtractMetadata results in deterministic sorted order (gosqlx: ExtractMetadata and Extract* return tables/columns/functions in random map order #619)
• bcf4393 fix(linter): make L006 tie-break and text-based L009 order deterministic (linter: L006 and text-based L009 results depend on map iteration order #618)

7 files changed, 350 insertions(+), 19 deletions(-)

Test Plan

• Each issue's reproduction cases were run against main before the fix (Haiku scout), and a failing regression test was written before the code change.
• Independent review (Opus, or Sonnet on small chains) with a bounded fix loop.
• Independent Opus validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding)), go vet, golangci-lint, re-ran the reproduction cases on HEAD, and confirmed the new tests fail on the parent commit.
• Perf items: benchstat -count=6 before/after on the same machine.

#618 evidence (validator)

Not a performance item, so no benchstat. Evidence that the bug is reproduced and fixed:
(1) On the parent commit 61ca6c8, I unpacked the parent into scratchpad/b3/618/parent with git archive and ran the new test files there with -count=5. All three new tests failed in every one of the 5 runs. TestColumnAlignmentRule_TieBreakDeterministic got [{2 3}] instead of [{3 5}], and [{2 5}] instead of [{3 3}] for the reversed tie. TestAliasingConsistencyRule_TextBasedOrderDeterministic failed in all 5 runs. TestLint_DeterministicL006L009 got [{L006 2 3}] instead of [{L006 3 5}], and [{L009 3 1} {L009 2 1}] instead of [{L009 2 1} {L009 3 1}].
(2) On HEAD, the same tests pass with -count=5 -race. Each one repeats 100 to 200 calls per subtest.
(3) CLI on HEAD: 100 runs of `gosqlx lint l006.sql` printed '[L006] Column Alignment at line 3, column 5' every time. 20 runs of `gosqlx lint l009.sql` printed L009 at line 2, column 1 then line 3, column 1 every time.

#618 race gate: I ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) on HEAD bcf4393. It exited 0 with 44 packages ok and no FAIL lines. I also ran go test -count=5 -race ./pkg/linter/rules/style/ ./pkg/gosqlx/ and both packages passed (style 1.2s, gosqlx 10.8s).
#619 evidence (validator)

benchstat on the implementer's before.txt vs after.txt (n=6, Apple M4), sec/op:
- ExtractMetadata: 1.517µs to 1.741µs (+14.77%, p=0.002)
- ExtractMetadataLarge: 34.15µs to 37.51µs (+9.84%)
- ExtractFunctions: 373.3ns to 412.9ns (+10.61%)
- ExtractTables: 266.9ns to 276.0ns (+3.39%)
- ExtractColumns: not significant (p=0.065)
- geomean: +10.14%

B/op and allocs/op are identical on every benchmark (for example ExtractMetadata stays at 1.547Ki and 22 allocs, Large at 471 allocs). The added time is the cost of the sort, which the issue allows. These are not tokenizer, parser or AST hot paths, and performance_baselines.json has no Extract entries.

Repro evidence:
- On HEAD, both scout queries were each run through ExtractMetadata plus the five individual extractors 30 times. Every field had exactly 1 distinct order.
- Issue SQL: Tables [t1 t2 t3], Columns [a b c d e f g h x y z], ColumnsQualified a..h, t1.x, t2.y, t3.z, Functions [].
- Schema/alias SQL: Tables [sch1.t1 sch2.t2 t3], Columns [a b c d e x y z], ColumnsQualified d, e, s1.a, s1.c, s1.x, s2.b, s2.y, t3.z, Functions [COUNT MAX SUM].
- Parent check: I unpacked the parent (bcf4393) with `git archive` into scratchpad/b3/619/parent and added the new test file. It does not compile there because compareQualifiedName does not exist yet. With only that helper added as a test shim, all 4 Issue619 subtests and all 3 SortedInvariant subtests FAIL on the parent.

#619 race gate: I ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) on HEAD 609fa8d. It exited 0: 44 packages ok, no FAIL lines, including pkg/gosqlx (5.9s), pkg/gosqlx/testing and pkg/mcp. CGO_ENABLED=1 go test ./pkg/cbinding/ also passed. The targeted race run of TestExtract_DeterministicOrder_Issue619, TestExtract_SortedInvariant, TestExtract_ResultsDoNotRetainInput and TestExtract_RetentionHarnessDetectsAliasing passed.

Breaking Changes

• linter: L006 and text-based L009 results depend on map iteration order #618: None to exported signatures. Observable behaviour is now deterministic: when indents tie, L006 picks the earliest continuation-column indent, and text-fallback L009 emits violations in first-seen table order. Both outcomes were random before. Violation counts, messages and lines are unchanged.
• gosqlx: ExtractMetadata and Extract* return tables/columns/functions in random map order #619: No exported signatures change. Observable behaviour changes: the slices returned by ExtractTables, ExtractTablesQualified, ExtractColumns, ExtractColumnsQualified, ExtractFunctions and ExtractMetadata (and therefore MCP extract_metadata, the cbinding JSON and the integration outputs built on them) are now sorted and deterministic instead of in random map order. String slices use ascending byte order; QualifiedName slices are ordered by Schema, then Table, then Name. This is an additive guarantee, now documented in godoc on Metadata and on each function.

Notes

• CHANGELOG.md is intentionally not touched; the consolidated CHANGELOG PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 gets the entries for this batch before it merges.
• Part of the batch-3 merge train (follow-ups cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599-schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620); later PRs in the train are rebased onto this one after it merges.
• Base: main.

Fixes #618
Fixes #619
Follow-up to #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 14, 2026 5:41am UTC

[github-actions]

Claude finished @ajitpratap0's task in 16s —— View job

---

Review of PR #625 (deterministic L006/L009 and Extract* ordering)

I read the diff of the three source files. I did not run the tests or the benchmarks, and I only skimmed the test files. The race-gate and benchstat numbers below come from the PR description.

Summary: The changes are small and correct, and I found no blocking issues.

pkg/gosqlx/extract.go (#619)

• slices.Sort and slices.SortFunc run before detachStrings and the arena copy. Sorting only reorders slice headers, so retention protection and allocs/op stay the same, which matches the identical B/op and allocs/op in the benchmarks.
• compareQualifiedName compares all three fields through cmp.Or, so the order is total and deterministic.
• The godoc and example outputs are updated to match the new sorted order. That is a good catch, since the old examples would now be wrong.
• The extra 3–15% ns/op is acceptable for these non-hot paths. If it ever matters, sorting only when len > 1 would save a call on the common single-item case, but slices.Sort already returns early for short slices.
• Behaviour change: callers who relied on the random order (MCP extract_metadata, cbinding JSON) now see sorted output. The PR documents this, and it is stricter than before, not looser. Because CHANGELOG.md is deferred to docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598, please make sure that PR gets an entry for it.

pkg/linter/rules/style/column_alignment.go (#618)

• Scanning indents[1:] in source order and using a strict > makes the earliest indent win ties. This is correct and deterministic.
• The [1:] slice is safe if the function still has a guard for len(indents) < 2 earlier. I only saw the tail of the function in the diff. Please confirm that guard exists, because indents[1:] panics on an empty slice.

pkg/linter/rules/style/aliasing_consistency.go (#618)

• tableOrder records the first sighting of each table name, and the check loop now iterates it. This is correct.
• Inside that loop, for alias := range definedAliases only computes a boolean hasAlias, so its random order cannot change the output. I still recommend checking the rest of the loop body for any message built from map iteration.
• Each key is appended once, so fullTableNames[tableName] always exists. Behaviour is the same as before, apart from the order.

Tests

• The PR description says the repeated-call tests (50–200 iterations) fail on the parent commit and pass with -race -count=5. That is the right way to test map-order bugs.
• Test coverage looks adequate. One optional addition is a test for mixed-case names, to pin the documented byte ordering (COUNT before count).

Follow-ups

• The cfg.Rules map iteration in pkg/linter/config/config.go is the same class of bug and gives cfg.Warnings a random order. Please open a tracking issue for it, as the PR notes.

Verdict: LGTM, with the indents[1:] guard check as the only item to confirm.

[ajitpratap0]

Merge train (batch 3): rebased onto main after #621 merged (ddd8521). The adversarial + security pass (Opus) found nothing. Answers to the bot review:

• indents[1:] guard: confirmed. column_alignment.go:163 returns early when len(indents) < 2, before any indents[1:] slice.
• aliasing_consistency.go: the inner definedAliases range only computes a boolean, so iteration order cannot reach the output.
• CHANGELOG: the entries for the sorted Extract* order and the deterministic L006/L009 results go into docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 before it merges.
• cfg.Rules warning order: will be filed as a follow-up issue with the rest of this batch's follow-ups.

[github-actions]

Claude finished @ajitpratap0's task in 14s —— View job

---

Review of PR #625 (deterministic L006/L009 and Extract* ordering)

I read the diffs of the source files and checked the one open item from the earlier review. I did not run the tests or benchmarks. The race-gate and benchstat numbers come from the PR description.

Verdict: LGTM. I found no blocking issues.

• indents[1:] guard confirmed. checkColumnAlignment returns early at column_alignment.go:163 when len(indents) < 2. The slice is therefore never empty, and the [1:] slice is safe. Scanning indents[1:] in source order with a strict > makes the earliest indent win ties, and the first column is still skipped.
• aliasing_consistency.go. tableOrder gets one entry per table name, appended only on first sight. Every key in fullTableNames is therefore iterated exactly once, in source order. The inner for alias := range definedAliases only sets a boolean, so its random order cannot affect output.
• extract.go. The sort runs before the detach/arena copy, so retention protection and allocs/op are unchanged. compareQualifiedName orders by Schema, then Table, then Name, which is a total order.
• Tests. The repeated-call tests (50–200 iterations) are the right way to catch map-order bugs. Optionally, add a mixed-case case such as COUNT vs count to pin the documented byte ordering.

Follow-ups

• Open an issue for the cfg.Rules map iteration in pkg/linter/config/config.go. It is the same class of bug and gives cfg.Warnings a random order.
• Make sure the consolidated CHANGELOG PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 notes that Extract* and ExtractMetadata results are now sorted. This also affects the MCP extract_metadata tool and the cbinding JSON output.