WIP [draft]: Admin Layer PR A — the rewrite (create) - #526
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The plan's clamp fixture carried an off-by-one: last_active_date 2026-09-04 is seven UTC days before the day under test, which lifecycle.py (>=8, since 3c05049) classifies onboarding, not at_risk. The fixture now uses the 3rd, matching the at_risk the test documents.
Comment on lines
+31
to
+38
| from dashboard.backend.domain.credits.repository import ( | ||
| _assemble_billing_states, | ||
| _assemble_commercial_ledger, | ||
| _assemble_ledger_day, | ||
| _day_bounds, | ||
| _unique_user_ids, | ||
| _utc_text, | ||
| ) |
The plan's absent-user fixture needed the user row to exist: the per-user reader lazily creates the credit_accounts row and its FK points at users, so comparing the two paths on a truly nonexistent id failed before any signal was computed. The agent-source ordering fixture now pins created_at — created_at has second resolution and the SQL tie-breaks on the random agent_id.
The list_daily_subjects insertion matched lazily up to the first 'return existing' — the early-exit inside the if, not the method end — which amputated the 500-batch loop from list_existing_source_event_ids on both twins.
The worker owns rollup_day and the retention coordinator (the reaper keeps only the two throttled snapshot repairs), runs the idempotent user_activity seed and the eight-week history copy once before its first tick, and its interval comes from ANALYTICS_DAILY_JOB_INTERVAL_SECONDS (default 300, junk-tolerant), stripped in conftest and documented in CLAUDE.md.
| with _worker_lock: | ||
| thread, stop = _worker_thread, _worker_stop | ||
| _worker_thread = None | ||
| _worker_stop = None |
| """Design SS6.9 step 1 / D23: the daily job owns rollup_day now, so the | ||
| reaper tick must not be able to write the same rollup rows.""" | ||
| parameters = inspect.signature(maintenance.run_analytics_maintenance).parameters | ||
| source = inspect.getsource(maintenance) |
MrParamecium
marked this pull request as ready for review
September 23, 2026 15:05
Collaborator
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b08ed2474
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+389
to
+392
| _compute_day( | ||
| stale_day, now=current, store=store, run_history_store=runs, rollup=rollup_fn | ||
| ) | ||
| recomputed.append(stale_day) |
There was a problem hiding this comment.
When a late-arrival recomputation fails, _compute_day returns an outcome with failed_steps, but that result is discarded and the date is still appended to recomputed_dates. This makes the report claim the stale fact was refreshed even though the old rows remain, and callers have no indication that the retry was unsuccessful; capture the outcome and report the failure (or leave the date out) so recomputation errors remain observable.
Useful? React with 👍 / 👎.
…redits import cycle, remove unreachable code and dead names - daily_facts: a failed stale-day recompute now releases the day lease and stays out of recomputed_dates (Codex P2) - credits: the six shared ledger helpers move to repository_common.py, killing the repository_postgres → repository import cycle CodeQL flagged - analytics twins: the dangling pre-repair loop bodies after list_daily_subjects (unreachable since the Task 9 fix) are removed - dead names: unused pytest import, unused module constant, duplicate test definition
Collaborator
Author
|
Review findings addressed in 78d5fc4: Codex P2 — surface failed stale-day recomputations: CodeQL findings:
All findings fixed; 1951 tests across the touched slices green. |
| from collections.abc import Iterator, Sequence | ||
| from contextlib import contextmanager | ||
| from datetime import datetime, timedelta | ||
| from datetime import date, datetime, timedelta, timezone |
| def test_maintenance_no_longer_owns_rollups_or_the_lifecycle_backfill(): | ||
| """Design SS6.9 step 1 / D23: the daily job owns rollup_day now, so the | ||
| reaper tick must not be able to write the same rollup rows.""" | ||
| parameters = inspect.signature(maintenance.run_analytics_maintenance).parameters |
The discipline guard forbids store calls inside loops; the Codex-P2 fix put release_projection_day inside the stale-day loop. The lease belongs to the job row, so one release after the loop carries the same retry semantics — failed days stay in list_days_needing_recompute and the next tick retries them.
This branch was successfully deployed
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
2 participants
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.
⛔ Draft until CI's Postgres tier is green on this branch — merge only after that and the checklist below.
Implements
docs/superpowers/plans/2026-09-15-admin-layer-redesign-prA-rewrite-create.md(14 tasks), the data model for design §6:user_activity,user_daily_facts,lifecycle_transitions,agent_runs.owner_user_id, the read-time lifecycle calculator, the two-field day claim, population-wide operational signals, and the daily job on its own worker thread. No response shape changes; no table dropped; no frontend file touched; nocohortadded.Acceptance checklist (plan §Acceptance)
user_activity,user_daily_facts(user_group DEFAULT 'unknown', nullable operational_reason_code, no cohort) andlifecycle_transitions(UNIQUE per user/day, data_quality) in both DDL constants, twin-parity column check green (Task 1)agent_runs.owner_user_idon both run-history twins, threaded from the authenticated caller through the four hops, NULL for callerless runs (Task 2)user_activitywith one MIN/MAX upsert per accepted lifecycle event, zero history reads; live singletonmaintain_activity=True,project_snapshots=False; seed idempotent, never regresses (Task 3)build_lifecycle_inputsclamps evidence toas_of, prefers the day's own activity, unrouted (Task 4)list_commercial_values/list_credit_activitybyte-identical through the owning stores (Task 5)backfill.pyhas no_get_connection/database_url; its_DIALECT_BRANCH_ALLOWLISTentry removed in the same commit; the four source reads exist on both twins (Task 6)list_operational_signalsagrees withget_operational_factson every seeded state, same call count at 10 and 40 users, no scalar user id;consecutive_failed_terminal_runssingle-owner (Task 7)run_daily_factswrites every §6.7 column, excludes admins/excluded/pre-D accounts, survives post-midnight activity, counts D in trailing window, transitions DO UPDATE with data_quality, partial-day retry, late-arrival recompute, retention coordinator per tick (Task 9)user_daily_factsaspartialrows, idempotent, never overwrites complete rows (Task 10)analytics-daily-factsthread,ANALYTICS_DAILY_JOB_INTERVAL_SECONDS(default 300, junk-tolerant), app.py starts it beside the reaper; retention moves off the reaper; maintenance keeps the two repairs (Task 11)user_activityuntouched;retention.pynever names the table (guard test + probe-verified teeth) (Task 12)_raise_service_errorlogs category-only and raises 503from exc;admin_analyticsinBLOCKING_IO_ROUTER_MODULES; dead users-list stack deleted; route set unchanged (Task 13)test_architecture_boundaries.py(Task 14)Verification
TEST_POSTGRES_URL): 5496 passed, 48 skipped, 0 failed (same skip count as the pre-branch baseline — all non-PG skips are pre-existing)._DUPLICATED_BODIES/_DIALECT_BRANCH_ALLOWLISTrespected (allowlist entry forvalue_repository.pyrewritten with the new reason; backfill's entry removed in the same commit as the fix).git add -Aused anywhere; seedbacktest.dbverified clean after every task.Deviations from the plan text (all cosmetic, none semantic)
last_active_date=2026-09-04is 7 days before the computed day — lifecycle.py (≥8 since 3c05049) classifiesonboarding, not the documentedat_risk. Fixture uses the 3rd.list_existing_source_event_ids" matches up to the firstreturn existing(the early exit), truncating the batch loop on both twins. Restored in a follow-up commit.test_admin_analytics_value_frontend.pyno longer exists (deleted by the feat: consolidate admin navigation on /admin #488 nav consolidation); Task 13's verification ran against the surviving conformance oracle (test_admin_analytics_frontend.py,test_app_composition.py).test_analytics_maintenance.pyandtest_run_lifecycle_unification.pycarried pre-existing source pins contradicting Task 11 (retention-on-reaper); rewritten per the plan's own replacement snippets.PR B cannot land until this merges and prod writes 8 days of facts — merged-to-main auto-deploys, so the clock starts at merge.