fix(sync): R5 — code↔DB sync reliability & correctness (closes 22 audit findings) - #177
Merged
Conversation
…tracts) Covers all 22 sync-audit findings (9 High, 9 Medium, 4 Low). Locks contracts for: parent-run heartbeat (H1), batch analysis name-reconciliation (H2/H3), all-fallback overwrite guard + confidence gate (H4), owner-attributed budget gate (H5), PII scrubber + opt-in flag (H6), is_indexed status whitelist (H7), IntegrityError->409 + model index parity (H8), adopt-not-run (H9), schema- qualified table identity (M2), and the Medium/Low set. Approach A (surgical + targeted structural). Branch + 5-wave plan defined. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Turns the R5 design spec into 18 bite-sized TDD tasks across 5 waves with a dependency graph and disjoint file ownership for parallel subagents. Each task: exact file:line anchors, complete code, failing-test-first, exact commands, conventional commit, DoD. Covers all 22 audit findings; T18 includes the per-finding business-logic validation cycle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s C1-C7 Adversarial re-validation (3 independent passes) confirmed the business-logic understanding for all 22 findings is correct (zero misreads). Applied binding corrections: C1 owner-less project degrades (not blocks) budget; C2 omit_samples threaded param not instance attr; C3 sync_now 429 step; C4 M3 parent workflow_id + targeted heartbeat UPDATE (no version lost-update); C5 validate ORM partial- index form + Alembic kwargs; C6 index required-filters under bare suffix for schema-qualified names; C7 wording. Plan Validation Log governs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…redaction (T2 review)
Implement sync_budget.py with C1 correction: missing owner degrades gracefully (unenforced) rather than blocking syncs for legacy/deleted-owner projects. - resolve_owner_user_id: query project owner from DB - build_sink: instantiate DbUsageSink with owner attribution - preflight_owner_budget: pre-flight check with C1 graceful degradation TDD: 3 tests, all passing. C1 applied: test_preflight_owner_missing asserts ok=True when owner is None (unenforced, not blocked). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + fallback marker (H2,H3,H4) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…budget wiring, op_kind heuristic label (M2,H4,H6,H5,H2,M7,L4) - M2: _match_tables now uses (schema, name) tuple key; schema-ambiguous tables get qualified display name and a NOTE in code_context - H4: all-fallback guard aborts Step 5 store when < sync_min_success_ratio of analyses are non-fallback, leaving previous sync rows intact - H6: _build_db_context gains scrub/omit_samples params; run() reads Connection.send_sample_data_to_llm and settings.sync_pii_scrubbing_enabled to set local vars threaded through _match_tables → _build_db_context - H5: budget preflight (preflight_owner_budget) + per-run DbUsageSink wired into run() before heartbeat; Step 6 summary skipped when sink.budget_exceeded() - H2: store loop guards mt is None with continue + warning instead of if-mt-else ternaries; removes phantom upserts for misnamed analyses - M7: _AMBIGUOUS_VERBS tuple extracted; process_/handle_/sync_/set_/add_/ register_ removed from _WRITE_VERBS and classified as "unknown" - L4: distinct values block emits (+N more) when vals > 15; sample_data block appends …[truncated] when truncated at 800 chars - _make_matched() static helper DRYs up _MatchedTable construction Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gs, gate prompt header (M6,L2)
…or qualified tables (H4,C6) - _load_required_filters_by_table: skip CodeDbSync entries with confidence_score < settings.sync_min_confidence_to_enforce_filters (H4); also index schema-qualified table names (e.g. analytics.orders) under their bare suffix (e.g. orders) so check_required_filters matches bare query references (C6) - _load_sync_filters_and_mappings: apply the same H4 confidence gate so low-confidence entries are also omitted from prompt guidance text - 6 unit tests: 3 H4 cases (low-conf excluded, sufficient-conf included, boundary=threshold passes) and 3 C6 cases (qualified indexed under both keys, qualified+low-conf excluded from both, unqualified not duplicated) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ema-qualified db_index, scrub db-index LLM egress (H7,M9,M2,H6) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…uired_filters (M6)
…model index parity (H8) - run_coordinator.start(): catch IntegrityError on commit, rollback session, re-query for the race-winner, raise RunAlreadyActiveError — prevents raw 500 on TOCTOU concurrent starts (R5) - indexing_run.py __table_args__: add uq_indexing_runs_active_one partial-unique Index (coalesce form, verified compiles + enforces in SQLite) matching prod migration a1f2b3c4d5e6 — unit tests now enforce single-active via real DB constraint, not just app-level pre-check - migration f37386df158c: idempotent create_index (if_not_exists) / drop_index (if_exists) parity for envs missing the original hotfix migration - test: H8 TOCTOU test confirms IntegrityError→RunAlreadyActiveError translation and session usability after rollback; TDD RED→GREEN cycle documented Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t skip, overview regen (H1,H9,M3,H5,M5) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The run_code_db_sync worker task was reading result.get("synced_tables")
but CodeDbSyncPipeline.run returns the key "synced", causing matched=None
in logs. Changed to result.get("synced") to fix the log output.
Added test_worker_sync.py with a test that patches the pipeline to return
the correct dict and asserts the log contains matched=2.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onConfig propagation + trigger_sync budget 429 (H6,H5) - Connection model: add send_sample_data_to_llm bool (default True, server_default=1) - Migration e909ec65d857: add column with batch-mode downgrade for SQLite - ConnectionConfig dataclass: add send_sample_data_to_llm field (default True) - ConnectionService.to_config: propagate flag from ORM row into ConnectionConfig - ConnectionCreate/Update/Response schemas: expose flag (not a secret) - _UPDATABLE_FIELDS: include send_sample_data_to_llm so PATCH persists it - trigger_sync: preflight_owner_budget gate before start-lock (429 when over budget) - Tests: 4 unit (model default/persist/to_config propagation) + 2 integration (429 gate) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onnections (M4,M1) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gression test (C3,M4) C3: Add preflight_owner_budget 429 gate to sync_now route (after role check, before coord.start). Reuses same pattern as trigger_sync (T14). Blocks over-budget owners before any state changes. M4: Add regression tests asserting get_sync_schedule next_run hour matches effective per-project hour. Tests confirm hourly cron contract is held (no drift between displayed hour and compute_next_scheduled_run). All 4 tests pass (budget gate 429, passthrough 202/409, hour consistency with override, global fallback). Ruff/mypy pass. No code changes needed to schedule logic—already correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…light, defensive owner lookup, migration no-op, update stale tests to new contracts (T18) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssues.md (T18) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
R5 — code↔DB sync reliability & correctness
Closes all 22 findings of the 2026-06-25 five-specialist code↔DB synchronization audit
(9 High · 9 Medium · 4 Low). Executed as 18 TDD tasks across 5 waves via subagent-driven
development; every task got a fresh implementer + a two-stage (spec + quality) review, and the
whole branch passed a final opus review (Ready to merge, 0 Critical / 0 Important).
docs/superpowers/specs/2026-06-25-sync-remediation-design.mddocs/superpowers/plans/2026-06-25-sync-remediation.mdGates (verified locally)
unit + integration: 4560 passed / 0 failedruff format+ruff check+mypyclean; alembicupgrade head/downgrade basecleanWhat changed
Reliability (High)
UPDATE, noversionlost-update) → the stale-run reaper no longer kills a healthy multi-minute sync.
RunCoordinator.starttranslates the single-activeIntegrityErrorinto a cleanRunAlreadyActiveError/409 with session rollback; the partial-unique active index is mirroredonto the model for
create_alltest parity.pipeline on an active-run conflict.
is_indexedcounts onlycompleted/completed_partial(a failed-only index is no longerreported as indexed).
Data correctness (High)
table_name, not tool-call position(ends silent cross-table misattribution).
confidence_scoredegrades only its own table instead of aborting the batch.longer enforce/surface required-filter guidance.
Cost & privacy (High)
exhaustion; cron degrades gracefully; ownerless projects run unenforced).
LLM egress at both the sync and db-index analyzers, with a per-connection
send_sample_data_to_llmopt-out (default on).Medium — M1 reconciler covers all connections · M2 schema-qualified table identity (no
cross-schema collapse) · M3 parent-run progress steps (no 0%→100%) · M4 cron honors per-project
schedule hour · M5 overview regen + worker log key · M6 enrichment payload validation + deep-merge
op_kindheuristics labelled non-authoritative · M8 freshnessdefaults +
sync_failedflag · M9get_index_ageNULL guard.Low — L1 reaper logs sweep on unknown rowcount · L2 prompt header no longer fabricates an
"analyzed" date · L3 child-run orphaning (covered by H1+H9) · L4 truncation markers + tightened
relevance matching.
Migrations (linear chain, single head
e909ec65d857)2317bf9d9126schema-qualifieddb_indexuniqueness (M2)f37386df158cindexing-run active-index parity (no-op; index owned by upstreama1f2b3c4d5e6)e909ec65d857connections.send_sample_data_to_llm(H6)Notes
day's run — matches the prior daily-cron model (no startup catch-up).
🤖 Generated with Claude Code