feat(blackbox): add per-call subscription attribution store - #799
Merged
Merged
Conversation
Collaborator
Author
|
🔴 HOLD — do not merge (Apollo, subs-ace lane). FleetReview on
Work returns to kanban card |
… upsert Three P1 findings from FleetReview on d7fdcac: - turn_api_calls PRIMARY KEY(turn_id, seq) accepted NULLs (SQLite allows NULL in non-INTEGER PK columns, and NULLs never collide) -> NOT NULL on both columns plus a boundary reject in insert_api_call. - sweep() cascaded call rows only through the selected turns, so a call whose parent insert_turn never landed outlived retention forever -> bounded parentless delete on the call's own ts, same transaction. - insert_turn used INSERT OR REPLACE, which deletes the row first and NULLed served_subs_json/attribution on any re-finalize -> INSERT ... ON CONFLICT DO UPDATE over the TurnRecord-owned columns only.
… not OR REPLACE The probe matched the literal 'INSERT OR REPLACE INTO turns', so the upsert rewrite made its mutation inert (caught by the probe's own self-check). Match the turns INSERT regardless of conflict strategy.
Collaborator
Author
FleetReviewConfidence: 2/5 Findings
FleetReview provenance · models: B=gpt-5.6-sol, C=claude-code-opus-5, F=gpt-5.6-sol, G=grok-4.6 · cost: $8.57 · duration: 23m 42s · rounds: 1 · files examined: 5 |
Collaborator
Author
FleetReviewConfidence: 3/5 Findings
FleetReview provenance · models: B=gpt-5.6-sol, C=claude-code-opus-5, F=gpt-5.6-sol, G=grok-4.6 · cost: $7.11 · duration: 32m 51s · rounds: 1 · files examined: 5 |
Parentless turn_api_calls rows are the normal state of an in-flight turn (calls are appended as they happen; the parent turns row only lands at finalize). Sweeping them on the retention cutoff alone deletes a live turn's call ledger whenever retention is short or the turn outruns it. Orphan deletion now requires the row to be past BOTH retention and _ORPHAN_GRACE_S (24h): orphan_cutoff = now - max(grace, retention). Verified: scripts/run_tests.sh tests/plugins/blackbox -q -> 15 files, 156 tests passed, 0 failed, EXIT=0. Mutation gate: replacing orphan_cutoff with the retention cutoff fails exactly test_orphan_sweep_respects_grace_period_independent_of_retention (1 failed, 12 passed).
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.
Scope
C1 store half only: guarded-additive turn_api_calls table and turns rollup columns, CanonicalUsage insertion, parent-retention cascade. No accumulator or transport changes; no live DB writes.
Verification
scripts/run_tests.sh tests/plugins/blackbox -q
15 files, 151 tests passed, 0 failed (100% complete) in 111.3s.
Initial per-call tests: 8 failed because insert_api_call was absent; then 8 passed.
AC7: 500 synthetic legacy rows copied using VACUUM INTO; actual frozen tokens.ace/journal/reprice SQL and unchanged cost/context readers produce byte-identical consumer output after migration and populated attribution. Repeated migration preserves SQL dump. Malformed INSERT control detects swallowed telemetry failure.
Review notes
Kyzcreig/hermes-agent resolves to ANG-Ventures/hermes-agent (verified via gh). No upstream port: Blackbox is fork-specific. AC7 covers consumer SQL/outputs, not a full external dashboard bake or separately executed old checkout.
Two SQL fixture EOF lines have whitespace flagged by git diff --check. Cleanup was refused by the committed-test guard, which misclassified this task worktree as live; left unchanged rather than bypass the guard.
Attribution validation rejects values outside wire/pinned/inferred/external; duplicate call keys raise instead of replacing records.