fix(monitor): bd monitor-commit-rate — detect no-ops by column comparison, not the frozen content_hash

[bourgois]

Carries forward the salvageable half of the closed #27, with its detection defect fixed.

Why this is a new branch and not a rebase of #27

#27 had two deliverables. Rebasing it onto post-resync main conflicted in internal/storage/issueops/update.go and internal/storage/domain/db/issue.go — and the conflicts were the tell:

Deliverable 1 (no-op-commit gate) is now upstream's. main has DiscardNoopIssueUpdates plus an early Changed: false return, which is exactly the ADR-0023 L-A semantics — a value-identical bd update writes nothing, so it mints no Dolt commit. Resolving those conflicts would have landed a second implementation of a gate upstream already has. Those two commits are dropped; only the monitor is cherry-picked here.

Deliverable 2 was broken. See below.

The defect

The watchdog decided "no-op" with from_content_hash == to_content_hash on dolt_diff_issues. But content_hash is written by the upsert/import path only (issueUpsertColumns in issueops/helpers.go) and is never recomputed by the update path — zero references in issueops/update.go.

So after any bd update --title X the hash is unchanged on both sides and compares equal. The single signal the command relied on cannot tell a real edit from a no-op, and the command would alert on routine fleet activity — inverted from its purpose.

Its own test caught it, and had never been run: the -run filter used when the file was committed matched only one of the three tests in it.

monitor_commit_rate_embedded_test.go:120:
  CommitCount = 5, want 0 (every commit changed real content)

The fix

Compare the actual content columns, NULL-safely (<=>), with three documented exclusions:

Excluded	Why
updated_at	rewritten by every update — it is what a no-op consists of
row_lock	rewritten by any status/assignee write (freshRowLock); no user-visible content
content_hash	unmaintained on the update path, as above

An INSERT is also no longer eligible — it added a bead that wasn't there — so from_id IS NOT NULL is now required.

The column list is discovered from information_schema, not hardcoded. The issues table gains columns regularly upstream, and a stale hardcoded list fails open: an unlisted column's change would be invisible and the row would count as a no-op. Discovery makes a new column meaningful by default, which is the safe direction to be wrong in.

Verification

All three tests pass under BEADS_TEST_EMBEDDED_DOLT=1, and the pair is discriminating rather than trivially green:

• TestAnalyzeCommitPatternsIgnoresDistinctContentAcrossManyBeads — was failing, now passes (real edits excluded)
• TestAnalyzeCommitPatternsDetectsNoOpStorm — still passes (genuine storms still caught)

Both can only hold together if the signal actually discriminates; a fix that merely made everything look non-no-op would break the second.

go build ./... and go vet ./cmd/bd clean.

Note on scope

With upstream now gating value-identical updates inside bd itself, this command's remaining value is as an independent backstop — it samples committed history, so it still catches a recurrence from a writer that isn't bd (a stray script or supervisor). That is what the original design said it was for, and it is still true; the difference is that bd's own write path is no longer the expected source.

[bourgois]

Closing — parking the branch rather than shipping. The detection fix here is correct and worth keeping, but the case for landing the command does not hold up against the environment.

Why

Every storm source we know of is already closed.

• internal/storage/embeddeddolt/store.go:874 carries the vp-on8s guard: an auto-commit is skipped when only operational columns (updated_at, heartbeat_at, lease_expires_at, row_lock) changed — the lease/heartbeat case that motivated this.
• Upstream's DiscardNoopIssueUpdates now gates value-identical updates inside bd itself.
• The fleet's own direct-SQL writes (gascity/internal/beads/bdstore.go:1216,1244) are CAS-guarded — WHERE status = 'in_progress' AND assignee = <expected> — so they only match rows they actually change. Not a no-op source.

Nothing calls it. Zero references to monitor-commit-rate anywhere in gascity. The CHANGELOG entry advertises a city-pack order that does not exist.

So the command would be fork-local code — plus a cmd/bd/main.go registration line to re-apply on every resync — watching for a condition three separate fixes already prevent, with no consumer.

What the review found, for whoever picks this up

Not the reason for closing, but worth recording. Eight defects in ~200 lines:

• Registered in main(), not init() — the in-process test harness never sees the command, so bd monitor-commit-rate is "unknown command" in every in-process test. It also lands after InitDefaultHelpCmd() and carries no GroupID.
• Unbounded history scan — dolt_diff_issues materializes a diff for the whole table history and WHERE to_commit_date > ? filters after that work. Elsewhere the repo uses commit-scoped dolt_diff(from, to, 'issues') (issueops/blocked_merge.go). At per-minute cadence against a long-lived rig DB this is O(history), not O(window). This is the one that would matter most in production.
• --dry-run returns exit 0 before any analysis — a scheduled caller branching on the exit code gets "clean" for a database mid-storm.
• Alert text still says "Content hash similarity" — describing the signal this very branch removed.
• Deletions misclassified (to_id IS NOT NULL drops removed rows, so a commit that deletes one bead and no-op-updates another still counts as all-no-op); beadsPerCommit is named the reciprocal of what it holds; CHANGELOG claims a help-text caveat the help text does not contain; likely gosec G201 on the interpolated predicate.

One review claim does not hold for this fleet: "cannot run in the default backend" is true of a stock bd init (embedded), but the live rigs run dolt_mode: server, and *DoltStore satisfies storage.RawDBAccessor (verified by compile-time assertion). Proxied mode routes through a DoltStore too. So it would have run where it was meant to run — embedded support is polish, not a prerequisite.

State

Branch feat/monitor-commit-rate is parked with the detection fix intact: no-ops are now identified by NULL-safe comparison of the actual content columns, discovered from information_schema, instead of the frozen content_hash. Anyone resuming starts from a correct signal rather than one that classified every real edit as a no-op.