FLAG-1 unified task_attempts schema (single canonical DDL + 3-shape migration)
User decision 2026-11-05: unified schema, rejecting the rehearsal runtime column-union workaround and a ledger rename. One definition (cloud.attempt_ledger.TASK_ATTEMPTS_DDL, 12 columns) shared by cloud.attempts and cloud.db.POSTGRES_SCHEMA; ensure builds the unified table fresh and migrates old resume, gap-3 ledger, and rehearsal shapes in place with rows intact. aggregate counts ledger rows only (outcome NULL = resume row, never counted). SQLite verified by 8 new tests; Postgres real-DB upgrade path explicitly NOT verified (see batch summary). Local commit only: no merge, no push, no deploy.
This commit is contained in:
1 parent
6d009af48f
commit
6a70775ad9
6 files changed
+467
-113
No files matched your search
+172
-73
@@ -67,10 +67,50 @@ Semantics (pinned by tests, not left to each caller):
|
||||
|
||||
Persistence notes:
|
||||
|
||||
- One table, one definition. ``task_attempts`` is defined ONCE
|
||||
below (``TASK_ATTEMPTS_DDL``, aliased as ``SQLITE_DDL`` /
|
||||
``PG_DDL``); ``cloud.attempts`` (the resume-history writer) and
|
||||
``cloud.db.POSTGRES_SCHEMA`` (the fresh-Postgres schema) both
|
||||
import this same string instead of carrying their own copy.
|
||||
The two historical shapes -- main's resume DDL (``kind`` /
|
||||
``interrupted_reason`` / ``checkpoint_round`` / ``result_text``
|
||||
/ ``error_text``) and gap 3's ledger DDL (``outcome`` /
|
||||
``is_defect`` / ``defect_category`` / ``corrects_attempt_no``)
|
||||
-- are superseded by this unified definition, which carries
|
||||
both column groups. The shared columns (``task_id``,
|
||||
``attempt_no``, ``created_at``) had identical meaning and type
|
||||
on both sides, so there is no semantic conflict to arbitrate:
|
||||
the union of the column sets IS the unified schema, with one
|
||||
deliberate relaxation (see below).
|
||||
- Row kinds in one table. A row is a ledger row iff ``outcome``
|
||||
is non-NULL (written by this module, outcome validated in
|
||||
Python before the insert); a row with ``outcome`` NULL is a
|
||||
resume-history row (written by ``cloud.attempts``). The two
|
||||
writers insert disjoint column groups and read by column name,
|
||||
so they share the table without overwriting each other's
|
||||
fields. ``aggregate_attempts`` counts ONLY ledger rows: a
|
||||
resume row (outcome NULL) is skipped, never counted as a
|
||||
defect-free first pass -- otherwise a task that was merely
|
||||
auto-resumed would silently gain a counted attempt.
|
||||
- The one deliberate relaxation: ``outcome`` is nullable in the
|
||||
unified DDL even though the ledger writer always supplies it.
|
||||
A ``NOT NULL`` on ``outcome`` would make the resume writer's
|
||||
insert (which has no outcome to report) fail on a fresh table,
|
||||
so the constraint lives in Python validation here, not in the
|
||||
DDL. ``is_defect`` keeps ``NOT NULL DEFAULT 0`` so a resume
|
||||
row that omits it still reads as "not a defect judgement".
|
||||
- SQLite: ``ensure_task_attempts_table`` runs
|
||||
``CREATE TABLE IF NOT EXISTS`` -- idempotent, safe to call on
|
||||
every write, and a no-op on a database that already has the
|
||||
table (including one that already has rows).
|
||||
``CREATE TABLE IF NOT EXISTS`` with the unified DDL, then
|
||||
migrates a pre-existing legacy table (old resume shape, gap-3
|
||||
ledger shape, or the rehearsal-era 12-column shape) by adding
|
||||
whichever unified columns it lacks. Added columns are always
|
||||
nullable or defaulted, so existing rows are never blocked;
|
||||
SQLite ``ALTER TABLE`` cannot tighten an existing column's
|
||||
nullability, so a migrated table keeps its looser column
|
||||
types -- the column SET and the data are unified, the
|
||||
``NOT NULL`` spelling on fresh tables is not retrofitted.
|
||||
This is a migration to the single canonical definition, not a
|
||||
second definition assembled at runtime.
|
||||
- Postgres: the production table is NOT created by
|
||||
``init_postgres_schema`` (deliberately: that schema is covered by
|
||||
the migrations test asserting ``MIGRATION_TABLES`` equals the
|
||||
@@ -87,7 +127,10 @@ Persistence notes:
|
||||
``test_attempt_ledger.py``). Until that manual step runs, the
|
||||
endpoint treats a missing table exactly like "no ledger rows":
|
||||
unknown rates, not an error -- the same fail-soft shape already
|
||||
used for missing ``cost_records``.
|
||||
used for missing ``cost_records``. The Postgres upgrade path
|
||||
for a legacy-shaped table (fresh ``PG_DDL`` vs. adding the
|
||||
missing unified columns) has NOT been verified against a real
|
||||
Postgres instance in this batch -- see the batch summary.
|
||||
- The read path (``GET /metrics-report``) never creates the table:
|
||||
a read-only endpoint must not mutate schema. Table creation
|
||||
happens only on the write path, inside ``record_attempt`` /
|
||||
@@ -104,18 +147,15 @@ Persistence notes:
|
||||
unknown until the hook is deliberately enabled. The worker hook
|
||||
always records ``is_defect=False``: it never judges a defect,
|
||||
so enabling it can never turn a transient retry into a defect.
|
||||
- Schema evolution note (pre-production, this track):
|
||||
``corrects_attempt_no`` was added to the DDL after audit gap 3
|
||||
but BEFORE either DDL reached production (gap 3 is still an
|
||||
OPEN, unmerged PR), so no production table needs migration:
|
||||
fresh tables get the column from the DDL below. A database
|
||||
created from gap 3's DDL alone (temporary/test databases only)
|
||||
gains the column with one statement --
|
||||
``ALTER TABLE task_attempts ADD COLUMN corrects_attempt_no
|
||||
INTEGER`` -- which is also the documented step if gap 3's DDL
|
||||
was ever run on production Postgres before this track lands.
|
||||
The read path uses ``SELECT *`` deliberately so a table without
|
||||
the column still reads (its rows aggregate as plain attempts).
|
||||
- Schema evolution note (unified schema, this batch):
|
||||
the old resume DDL, the gap-3 ledger DDL, and the rehearsal
|
||||
12-column shape are all legacy shapes of the SAME table.
|
||||
``ensure_task_attempts_table`` migrates any of them to the
|
||||
unified column set in place (add missing columns only, never
|
||||
drop or rewrite rows); the read path uses ``SELECT *``
|
||||
deliberately so rows of any shape still read, and
|
||||
``aggregate_attempts`` treats a row without an outcome as a
|
||||
resume row, not as a ledger attempt (see above).
|
||||
|
||||
This module imports no ``cloud.*`` module and opens no database
|
||||
connection itself: callers pass a connection in, so the layer rules
|
||||
@@ -130,38 +170,92 @@ TABLE = "task_attempts"
|
||||
|
||||
OUTCOMES = frozenset({"success", "failure", "interrupted"})
|
||||
|
||||
# One DDL serves both backends: every type and the composite key are
|
||||
# valid in SQLite and in Postgres, and there is no AUTOINCREMENT or
|
||||
# identity column for either backend's sequence handling to chase.
|
||||
# ``corrects_attempt_no`` (NULL = a real attempt row; non-NULL = a
|
||||
# correction row for that attempt_no, never counted as an attempt)
|
||||
# was added by the lifecycle-hook track; it SUPERSEDES gap 3's DDL
|
||||
# for any production manual step (gap 3 never merged, so no
|
||||
# production table exists yet -- see the module docstring).
|
||||
SQLITE_DDL = (
|
||||
# THE single canonical definition of task_attempts (unified
|
||||
# schema, FLAG-1 decision 2026-11-05: "unified schema", rejecting
|
||||
# both the rehearsal-era runtime column-union workaround and a
|
||||
# ledger rename). One DDL serves both backends: every type and
|
||||
# the composite key are valid in SQLite and in Postgres, and
|
||||
# there is no AUTOINCREMENT or identity column for either
|
||||
# backend's sequence handling to chase.
|
||||
#
|
||||
# Column groups, in order (each group's internal order preserves
|
||||
# its historical DDL order):
|
||||
# - shared: task_id, attempt_no (composite PK), created_at last;
|
||||
# - ledger (gap 3 + lifecycle hook): outcome, is_defect,
|
||||
# defect_category, corrects_attempt_no (NULL = a real attempt
|
||||
# row; non-NULL = a correction row for that attempt_no, never
|
||||
# counted as an attempt);
|
||||
# - resume (main v6): kind, interrupted_reason,
|
||||
# checkpoint_round, result_text, error_text.
|
||||
# ``outcome`` is deliberately nullable (see the module docstring:
|
||||
# the resume writer has no outcome to report, so a NOT NULL here
|
||||
# would break its insert; the ledger writer validates outcome in
|
||||
# Python instead). ``cloud.attempts.CREATE_TABLE_SQL`` and the
|
||||
# ``task_attempts`` entry of ``cloud.db.POSTGRES_SCHEMA`` are
|
||||
# this same string object, imported -- never a second copy.
|
||||
TASK_ATTEMPTS_DDL = (
|
||||
"CREATE TABLE IF NOT EXISTS task_attempts("
|
||||
"task_id TEXT NOT NULL, "
|
||||
"attempt_no INTEGER NOT NULL, "
|
||||
"outcome TEXT NOT NULL, "
|
||||
"outcome TEXT, "
|
||||
"is_defect INTEGER NOT NULL DEFAULT 0, "
|
||||
"defect_category TEXT, "
|
||||
"created_at DOUBLE PRECISION, "
|
||||
"corrects_attempt_no INTEGER, "
|
||||
"kind TEXT, "
|
||||
"interrupted_reason TEXT, "
|
||||
"checkpoint_round INTEGER, "
|
||||
"result_text TEXT, "
|
||||
"error_text TEXT, "
|
||||
"created_at DOUBLE PRECISION, "
|
||||
"PRIMARY KEY (task_id, attempt_no))"
|
||||
)
|
||||
|
||||
SQLITE_DDL = TASK_ATTEMPTS_DDL
|
||||
|
||||
# Production Postgres: run this verbatim by hand and verify the
|
||||
# table exists before expecting real rates from a Postgres-backed
|
||||
# endpoint. It is intentionally NOT part of POSTGRES_SCHEMA (see
|
||||
# the module docstring: migrations coverage must not widen silently).
|
||||
# This is the NEW DDL (with ``corrects_attempt_no``): if gap 3's
|
||||
# older DDL was ever run first, follow it with
|
||||
# ``ALTER TABLE task_attempts ADD COLUMN corrects_attempt_no
|
||||
# INTEGER`` and verify before expecting corrections to work.
|
||||
PG_DDL = SQLITE_DDL
|
||||
# For a legacy-shaped table already on Postgres, add whichever
|
||||
# unified columns are missing (see UNIFIED_COLUMN_TYPES);
|
||||
# that Postgres upgrade path is NOT yet verified against a real
|
||||
# Postgres instance -- see the batch summary.
|
||||
PG_DDL = TASK_ATTEMPTS_DDL
|
||||
|
||||
# One-line upgrade for a table created from gap 3's DDL (no
|
||||
# ``corrects_attempt_no`` column yet). Valid in both backends.
|
||||
# Unified column set, in DDL order. The migration in
|
||||
# ``ensure_task_attempts_table`` adds exactly these columns when
|
||||
# missing; the ALTER types below are the migration spellings
|
||||
# (nullable/defaulted, so existing rows are never blocked --
|
||||
# SQLite ALTER cannot add a NOT NULL column without a default
|
||||
# to a table that already has rows, and cannot tighten an
|
||||
# existing column at all).
|
||||
UNIFIED_COLUMNS = (
|
||||
"task_id", "attempt_no", "outcome", "is_defect",
|
||||
"defect_category", "corrects_attempt_no", "kind",
|
||||
"interrupted_reason", "checkpoint_round", "result_text",
|
||||
"error_text", "created_at",
|
||||
)
|
||||
|
||||
UNIFIED_COLUMN_TYPES = {
|
||||
"task_id": "TEXT",
|
||||
"attempt_no": "INTEGER",
|
||||
"outcome": "TEXT",
|
||||
"is_defect": "INTEGER DEFAULT 0",
|
||||
"defect_category": "TEXT",
|
||||
"corrects_attempt_no": "INTEGER",
|
||||
"kind": "TEXT",
|
||||
"interrupted_reason": "TEXT",
|
||||
"checkpoint_round": "INTEGER",
|
||||
"result_text": "TEXT",
|
||||
"error_text": "TEXT",
|
||||
"created_at": "DOUBLE PRECISION",
|
||||
}
|
||||
|
||||
# Legacy one-line upgrade for a table created from gap 3's DDL
|
||||
# (no ``corrects_attempt_no`` column yet). Retained for
|
||||
# backward compatibility with existing callers/tests; it is a
|
||||
# single step of the general migration above, not a second
|
||||
# schema definition. Valid in both backends.
|
||||
OLD_TABLE_ALTER = (
|
||||
"ALTER TABLE task_attempts "
|
||||
"ADD COLUMN corrects_attempt_no INTEGER"
|
||||
@@ -179,39 +273,34 @@ def _existing_columns(conn, table):
|
||||
|
||||
|
||||
def ensure_task_attempts_table(conn):
|
||||
"""Create the ledger table if absent. Idempotent; does not
|
||||
touch existing rows. The caller owns the commit (``record_attempt``
|
||||
"""Create the unified table if absent; migrate a legacy
|
||||
table to it if present. Idempotent; does not touch existing
|
||||
rows. The caller owns the commit (``record_attempt``
|
||||
commits; a bare ensure inside a larger transaction does not).
|
||||
|
||||
演练注 FLAG(合入语义取舍点,主结论见演练报告):
|
||||
main 侧 init_db/worker 用 cloud.attempts.py 的旧 DDL 建同名
|
||||
task_attempts 表(kind/interrupted_reason/checkpoint_round/
|
||||
result_text/error_text,无 outcome 系列);本模块的 CREATE
|
||||
TABLE IF NOT EXISTS 在旧表已存在时是空操作,之后按台账
|
||||
列写行即报 no column named outcome。本解法把表升级为两
|
||||
组列的并集:旧表先建时补台账 4 列,台账表先建时补旧 5
|
||||
列(新增列一律可空/带默认值,绝不对已有行加 NOT NULL,
|
||||
否则旧行会挡住 ALTER)。两边读路径均按列名取数,互不
|
||||
干扰;但这只是演练期保守解法——真合前须用户定:并集表、
|
||||
台账改名、或统一 schema 三选一,不可悄悄定为并集表。
|
||||
Unified schema (FLAG-1, user decision 2026-11-05): there is
|
||||
exactly one ``task_attempts`` definition
|
||||
(``TASK_ATTEMPTS_DDL``). A fresh database gets it verbatim
|
||||
from ``CREATE TABLE IF NOT EXISTS``. A database whose table
|
||||
was built from any legacy shape -- the old resume DDL, the
|
||||
gap-3 ledger DDL, or the rehearsal-era 12-column shape --
|
||||
is migrated in place by adding whichever unified columns it
|
||||
lacks (``UNIFIED_COLUMN_TYPES``); added columns are always
|
||||
nullable or defaulted, so existing rows are never blocked,
|
||||
and no row is dropped or rewritten. This replaces the
|
||||
rehearsal-era runtime column-completion workaround: the
|
||||
target here is the single canonical definition, and
|
||||
``cloud.attempts`` / ``cloud.db`` share that same string
|
||||
instead of each carrying a DDL of their own.
|
||||
"""
|
||||
conn.execute(SQLITE_DDL)
|
||||
conn.execute(TASK_ATTEMPTS_DDL)
|
||||
existing = _existing_columns(conn, "task_attempts")
|
||||
for col, ddl in (
|
||||
("outcome", "TEXT"),
|
||||
("is_defect", "INTEGER DEFAULT 0"),
|
||||
("defect_category", "TEXT"),
|
||||
("corrects_attempt_no", "INTEGER"),
|
||||
("kind", "TEXT"),
|
||||
("interrupted_reason", "TEXT"),
|
||||
("checkpoint_round", "INTEGER"),
|
||||
("result_text", "TEXT"),
|
||||
("error_text", "TEXT")):
|
||||
for col in UNIFIED_COLUMNS:
|
||||
if col not in existing:
|
||||
try:
|
||||
conn.execute(
|
||||
"ALTER TABLE task_attempts ADD COLUMN %s %s"
|
||||
% (col, ddl))
|
||||
% (col, UNIFIED_COLUMN_TYPES[col]))
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
@@ -272,11 +361,11 @@ def record_correction(conn, task_id, attempt_no, is_defect,
|
||||
correction row, a target whose outcome is ``interrupted``
|
||||
(interrupted rows are never defects, so there is no defect
|
||||
judgement to correct), and a defect category without a defect
|
||||
flag all raise ``ValueError`` and write nothing. On a table
|
||||
created from gap 3's DDL (no ``corrects_attempt_no`` column),
|
||||
the INSERT raises the backend's OperationalError loudly --
|
||||
run ``OLD_TABLE_ALTER`` first (see the module docstring);
|
||||
a correction must never silently degrade into a plain row.
|
||||
flag all raise ``ValueError`` and write nothing. A table
|
||||
created from a legacy DDL is migrated by ``ensure`` first
|
||||
(above), so the ``corrects_attempt_no`` column is present
|
||||
before the INSERT runs; a correction must never silently
|
||||
degrade into a plain row.
|
||||
Returns the correction row's own ``attempt_no``.
|
||||
"""
|
||||
if not task_id:
|
||||
@@ -325,12 +414,13 @@ def read_attempt_rows(conn):
|
||||
OperationalError when the table does not exist yet; the
|
||||
endpoint caller treats that as "no ledger", not as a failure.
|
||||
|
||||
``SELECT *`` is deliberate: a table created from gap 3's DDL
|
||||
has no ``corrects_attempt_no`` column, and naming that column
|
||||
here would turn those readable rows into an endpoint error.
|
||||
``SELECT *`` is deliberate: a legacy-shaped table may lack
|
||||
columns the unified DDL has, and naming a missing column
|
||||
here would turn readable rows into an endpoint error.
|
||||
With ``*`` the rows still read, and ``aggregate_attempts``
|
||||
treats a missing ``corrects_attempt_no`` as a plain attempt
|
||||
row (see ``_field_optional``).
|
||||
row (see ``_field_optional``) and a missing/NULL ``outcome``
|
||||
as a resume row, not a ledger attempt.
|
||||
"""
|
||||
return conn.execute(
|
||||
"SELECT * FROM task_attempts "
|
||||
@@ -364,10 +454,14 @@ def aggregate_attempts(rows):
|
||||
|
||||
Interrupted rows are excluded from both counts, and a task
|
||||
with only interrupted rows is omitted entirely (no counted
|
||||
attempt => its rates stay unknown, never 0). Correction rows
|
||||
(``corrects_attempt_no`` set) are excluded from the attempt
|
||||
count and instead OVERWRITE the defect judgement of their
|
||||
target attempt, in attempt order, last-wins: a task's
|
||||
attempt => its rates stay unknown, never 0). Resume rows
|
||||
(``outcome`` NULL -- written by ``cloud.attempts``, which
|
||||
records an interrupted attempt's history, not a ledger
|
||||
outcome) are likewise excluded: they are not ledger attempts
|
||||
and must never inflate a task's counted attempts. Correction
|
||||
rows (``corrects_attempt_no`` set) are excluded from the
|
||||
attempt count and instead OVERWRITE the defect judgement of
|
||||
their target attempt, in attempt order, last-wins: a task's
|
||||
``defects`` is the number of its counted attempts whose
|
||||
judgement AFTER corrections is a defect, and its
|
||||
``defect_category`` is the category of its FIRST defect
|
||||
@@ -392,7 +486,12 @@ def aggregate_attempts(rows):
|
||||
bool(_field(row, "is_defect")),
|
||||
_field(row, "defect_category")]
|
||||
continue
|
||||
if _field(row, "outcome") == "interrupted":
|
||||
outcome = _field_optional(row, "outcome")
|
||||
if outcome is None:
|
||||
# Resume-history row (cloud.attempts): no ledger
|
||||
# outcome, so not a counted ledger attempt.
|
||||
continue
|
||||
if outcome == "interrupted":
|
||||
continue
|
||||
entries = judged.setdefault(task_id, {})
|
||||
entries[_field(row, "attempt_no")] = [
|
||||
|
||||
Reference in new issue
Block a user