daemon: persist engine progress/probe to the store — segments:0 no longer leaks

Engine numbers never reached the store: download.get after a correctly-finished
download reported sizeBytes: null, downloadedBytes: 0, speedBps: 0, resumable:
false, segments: 0, segmentDetail: [] — segments: 0 breaks the frozen contract
(TaskSummary.segments is minimum:1, required).

- Scheduler::tick() now probes (EnginePort::probe) before every start(), persisting
  sizeBytes/resumable/validators via Tasks::set_probe_result before a byte moves,
  then starts the engine with that ProbeResult as probe_hint.
- Scheduler::persist_progress() (new) writes downloadedBytes/speedBps/segments/
  segmentDetail from the engine's Progress. Called from progress_snapshot() (the
  250ms tick) *and* once more from on_engine_state right before release()/unmap on
  every terminal transition, so a task that finishes between two ticks — the common
  case for anything small or fast — still leaves real numbers instead of the
  pre-persistence defaults.
- TaskSummary.segments is sourced from segments.size() when the task has any
  (matching what actually lands in segmentDetail, per the schema's "exactly
  segments entries"), falling back to the engine's effective_segments (budget
  slots *held*, not necessarily physical range count) only pre-segmentation.
- Tasks::set_final_bytes tops up on_finished's byte count as a last-resort
  backstop.
- store/segments.{cpp,hpp}: read/write access to the segments table behind
  TaskDetail.segmentDetail. Wired into daemon/CMakeLists.txt.
- migrations/0002: speed_bps on tasks and segments; fixes segments.state's CHECK
  to include 'pending' (0001 omitted it, so a pre-connect snapshot could never be
  written).
- store_migrations_test's forward-only loop faked "released version N" by setting
  the user_version pragma alone, with no real schema underneath — never exercised
  until 0002 existed. Fixed to actually build the db through migrations 1..N first.

Verified against real veloxd + tools/testserver (not just unit tests):
download.list/download.get correct immediately after completion and after a
daemon restart, with saveTo.allowedRoots pointed at an isolated dir.

Observed but not fixed (CORE, not this lane, noted in deferrals.md): Progress.
speed_bps reads back 0 for the whole lifetime of a live throttled download in the
same E2E check, despite downloadedBytes visibly advancing. DAEMON passes it
through unmodified; filed rather than worked around.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GRDjHGgpYmMoPE2UFbe7pP

daemon/src/store/migrations/0002_segment_speed_and_pending.sql

@@ -0,0 +1,30 @@
+-- Migration 0002 — segments gets a speed_bps column and a corrected state CHECK;
+-- tasks gets a speed_bps column too.
+--
+-- 0001's segments.state CHECK omitted 'pending' (vdm::segment::SegState::idle's wire
+-- spelling — "range assigned, no worker connected yet"), so a segment snapshot taken
+-- before its first worker connects could never be written. SQLite cannot ALTER a CHECK
+-- constraint in place, so this rebuilds the table (standard SQLite pattern: create the
+-- new shape, copy, drop, rename). speed_bps on both tables backs TaskSummary.speedBps /
+-- Segment.speedBps on the wire; absent from 0001 because progress writing wasn't wired
+-- yet — a task's engine-reported aggregate speed had nowhere to persist between polls.
+
+ALTER TABLE tasks ADD COLUMN speed_bps INTEGER NOT NULL DEFAULT 0;
+
+CREATE TABLE segments_new (
+    task_id         TEXT NOT NULL REFERENCES tasks(task_id) ON DELETE CASCADE,
+    idx             INTEGER NOT NULL,
+    start_byte      INTEGER NOT NULL,
+    end_byte        INTEGER NOT NULL,
+    completed_bytes INTEGER NOT NULL DEFAULT 0,
+    speed_bps       INTEGER NOT NULL DEFAULT 0,
+    state           TEXT NOT NULL DEFAULT 'pending'
+        CHECK (state IN ('pending','connecting','downloading','stalled','complete','failed')),
+    PRIMARY KEY (task_id, idx)
+) STRICT, WITHOUT ROWID;
+
+INSERT INTO segments_new (task_id, idx, start_byte, end_byte, completed_bytes, state)
+    SELECT task_id, idx, start_byte, end_byte, completed_bytes, state FROM segments;
+
+DROP TABLE segments;
+ALTER TABLE segments_new RENAME TO segments;

daemon/src/store/segments.cpp

@@ -0,0 +1,61 @@
+#include "store/segments.hpp"
+
+namespace velox::daemon::store {
+
+namespace proto = velox::proto;
+
+DbResult<void> Segments::replace_all(std::string_view task_id,
+                                     const std::vector<SegmentSnapshot>& segs) {
+    return db_.transaction([&]() -> DbResult<void> {
+        {
+            auto del = db_.prepare("DELETE FROM segments WHERE task_id = ?1");
+            if (!del) return std::unexpected(del.error());
+            if (auto r = del->bind(1, task_id); !r) return std::unexpected(r.error());
+            if (auto r = del->step(); !r) return std::unexpected(r.error());
+        }
+        for (const auto& s : segs) {
+            auto ins = db_.prepare(
+                "INSERT INTO segments(task_id, idx, start_byte, end_byte, completed_bytes, "
+                "speed_bps, state) VALUES(?1,?2,?3,?4,?5,?6,?7)");
+            if (!ins) return std::unexpected(ins.error());
+            if (auto r = ins->bind(1, task_id); !r) return std::unexpected(r.error());
+            if (auto r = ins->bind(2, static_cast<std::int64_t>(s.index)); !r)
+                return std::unexpected(r.error());
+            if (auto r = ins->bind(3, s.start_byte); !r) return std::unexpected(r.error());
+            if (auto r = ins->bind(4, s.end_byte); !r) return std::unexpected(r.error());
+            if (auto r = ins->bind(5, s.completed_bytes); !r) return std::unexpected(r.error());
+            if (auto r = ins->bind(6, s.speed_bps); !r) return std::unexpected(r.error());
+            if (auto r = ins->bind(7, std::string_view(s.state)); !r)
+                return std::unexpected(r.error());
+            if (auto r = ins->step(); !r) return std::unexpected(r.error());
+        }
+        return {};
+    });
+}
+
+DbResult<std::vector<proto::Segment>> Segments::list(std::string_view task_id) {
+    auto st = db_.prepare(
+        "SELECT idx, start_byte, end_byte, completed_bytes, speed_bps, state "
+        "FROM segments WHERE task_id = ?1 ORDER BY idx");
+    if (!st) return std::unexpected(st.error());
+    if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error());
+
+    std::vector<proto::Segment> out;
+    for (;;) {
+        auto row = st->step();
+        if (!row) return std::unexpected(row.error());
+        if (!*row) break;
+
+        proto::Segment seg;
+        seg.index = st->column_int(0);
+        seg.startByte = st->column_int(1);
+        seg.endByte = st->column_int(2);
+        seg.downloadedBytes = st->column_int(3);
+        seg.speedBps = st->column_int(4);
+        if (auto s = proto::parse_SegmentState(st->column_text(5))) seg.state = *s;
+        out.push_back(seg);
+    }
+    return out;
+}
+
+}  // namespace velox::daemon::store

daemon/src/store/segments.hpp

@@ -0,0 +1,45 @@
+#pragma once
+
+// Read/write access to the `segments` table — the per-connection detail behind
+// TaskDetail.segmentDetail (download.get) and the segment-bars widget. Written from an
+// engine progress tick (Scheduler::progress_snapshot); read back on download.get.
+
+#include <cstdint>
+#include <string>
+#include <string_view>
+#include <vector>
+
+#include "store/sqlite.hpp"
+#include "velox_proto.hpp"
+
+namespace velox::daemon::store {
+
+// One segment's live counters, as read off vdm::task::SegmentProgress. `state` is a wire
+// SegmentState spelling ("pending"/"connecting"/.../"failed") — the caller maps the
+// engine's own enum, this module stays generic like the others.
+struct SegmentSnapshot {
+    std::uint32_t index;
+    std::int64_t start_byte;
+    std::int64_t end_byte;    // inclusive (ADR 0010)
+    std::int64_t completed_bytes;
+    std::int64_t speed_bps;
+    std::string state;
+};
+
+class Segments {
+public:
+    explicit Segments(Db& db) : db_(db) {}
+
+    // Replaces every row for `task_id` with `segs` in one transaction. Simpler than a
+    // per-index upsert and cheap at <=32 rows, <=4 Hz.
+    DbResult<void> replace_all(std::string_view task_id, const std::vector<SegmentSnapshot>& segs);
+
+    // In index order. Empty if the task has never been segmented (TaskDetail's own
+    // description permits this).
+    DbResult<std::vector<velox::proto::Segment>> list(std::string_view task_id);
+
+private:
+    Db& db_;
+};
+
+}  // namespace velox::daemon::store

daemon/src/store/tasks.cpp

@@ -19,7 +19,7 @@ constexpr const char* kCols =
     "size_bytes, downloaded_bytes, resumable, "
     "req_segments, eff_segments, req_buffer_bytes, eff_buffer_bytes, queue_position, "
     "error_code, error_message, error_http_status, error_retryable, error_attempt, "
-    "error_next_retry_at";
+    "error_next_retry_at, speed_bps";
 
 DbResult<void> bind_opt(Stmt& s, int i, const std::optional<std::string>& v) {
     return v ? s.bind(i, std::string_view(*v)) : s.bind_null(i);
@@ -72,6 +72,7 @@ TaskRow read_row(Stmt& s) {
     if (!s.column_is_null(30)) r.error_retryable = s.column_int(30) != 0;
     r.error_attempt = col_opt_int(s, 31);
     r.error_next_retry_at = col_opt_text(s, 32);
+    r.speed_bps = s.column_int(33);
     return r;
 }
 
@@ -103,9 +104,9 @@ DbResult<void> Tasks::insert(const TaskRow& r) {
         "checksum_algo, checksum_value, size_bytes, downloaded_bytes, resumable, "
         "req_segments, eff_segments, req_buffer_bytes, eff_buffer_bytes, queue_position, "
         "error_code, error_message, error_http_status, error_retryable, error_attempt, "
-        "error_next_retry_at) VALUES("
+        "error_next_retry_at, speed_bps) VALUES("
         "?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15,?16,?17,?18,?19,?20,?21,?22,"
-        "?23,?24,?25,?26,?27,?28,?29,?30,?31,?32,?33)");
+        "?23,?24,?25,?26,?27,?28,?29,?30,?31,?32,?33,?34)");
     if (!st) return std::unexpected(st.error());
 
     auto chk = [](DbResult<void> r) { return r.has_value(); };
@@ -134,7 +135,8 @@ DbResult<void> Tasks::insert(const TaskRow& r) {
               chk(r.error_retryable ? st->bind(31, static_cast<std::int64_t>(*r.error_retryable))
                                     : st->bind_null(31)) &&
               chk(bind_opt(*st, 32, r.error_attempt)) &&
-              chk(bind_opt(*st, 33, r.error_next_retry_at));
+              chk(bind_opt(*st, 33, r.error_next_retry_at)) &&
+              chk(st->bind(34, r.speed_bps));
     if (!ok) return std::unexpected(DbError{0, "failed to bind a task column"});
 
     if (auto r2 = st->step(); !r2) return std::unexpected(r2.error());
@@ -265,15 +267,45 @@ DbResult<bool> Tasks::remove(std::string_view task_id) {
 }
 
 DbResult<bool> Tasks::update_progress(std::string_view task_id, std::int64_t downloaded_bytes,
-                                      std::int64_t eff_segments, std::int64_t eff_buffer_bytes) {
+                                      std::int64_t speed_bps, std::int64_t eff_segments,
+                                      std::int64_t eff_buffer_bytes) {
     auto st = db_.prepare(
-        "UPDATE tasks SET downloaded_bytes = ?2, eff_segments = ?3, eff_buffer_bytes = ?4 "
-        "WHERE task_id = ?1");
+        "UPDATE tasks SET downloaded_bytes = ?2, speed_bps = ?3, eff_segments = ?4, "
+        "eff_buffer_bytes = ?5 WHERE task_id = ?1");
     if (!st) return std::unexpected(st.error());
     if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error());
     if (auto r = st->bind(2, downloaded_bytes); !r) return std::unexpected(r.error());
-    if (auto r = st->bind(3, eff_segments); !r) return std::unexpected(r.error());
-    if (auto r = st->bind(4, eff_buffer_bytes); !r) return std::unexpected(r.error());
+    if (auto r = st->bind(3, speed_bps); !r) return std::unexpected(r.error());
+    if (auto r = st->bind(4, eff_segments); !r) return std::unexpected(r.error());
+    if (auto r = st->bind(5, eff_buffer_bytes); !r) return std::unexpected(r.error());
+    if (auto r = st->step(); !r) return std::unexpected(r.error());
+    return sqlite3_changes(db_.raw()) > 0;
+}
+
+DbResult<bool> Tasks::set_probe_result(std::string_view task_id, const ProbeFields& f) {
+    auto st = db_.prepare(
+        "UPDATE tasks SET size_bytes = ?2, resumable = ?3, etag = ?4, last_modified = ?5, "
+        "content_type = ?6, effective_url = ?7 WHERE task_id = ?1");
+    if (!st) return std::unexpected(st.error());
+    if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error());
+    if (auto r = bind_opt(*st, 2, f.size_bytes); !r) return std::unexpected(r.error());
+    if (auto r = st->bind(3, static_cast<std::int64_t>(f.resumable)); !r)
+        return std::unexpected(r.error());
+    if (auto r = bind_opt(*st, 4, f.etag); !r) return std::unexpected(r.error());
+    if (auto r = bind_opt(*st, 5, f.last_modified); !r) return std::unexpected(r.error());
+    if (auto r = bind_opt(*st, 6, f.content_type); !r) return std::unexpected(r.error());
+    if (auto r = bind_opt(*st, 7, f.effective_url); !r) return std::unexpected(r.error());
+    if (auto r = st->step(); !r) return std::unexpected(r.error());
+    return sqlite3_changes(db_.raw()) > 0;
+}
+
+DbResult<bool> Tasks::set_final_bytes(std::string_view task_id, std::int64_t bytes) {
+    auto st = db_.prepare(
+        "UPDATE tasks SET downloaded_bytes = ?2, size_bytes = COALESCE(size_bytes, ?2) "
+        "WHERE task_id = ?1");
+    if (!st) return std::unexpected(st.error());
+    if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error());
+    if (auto r = st->bind(2, bytes); !r) return std::unexpected(r.error());
     if (auto r = st->step(); !r) return std::unexpected(r.error());
     return sqlite3_changes(db_.raw()) > 0;
 }
@@ -296,9 +328,16 @@ proto::TaskSummary to_summary(const TaskRow& r) {
     s.sizeBytes = r.size_bytes;
     s.downloadedBytes = r.downloaded_bytes;
     if (auto st = proto::parse_TaskState(r.state)) s.state = *st;
-    s.speedBps = 0;
+    s.speedBps = r.speed_bps;
     s.resumable = r.resumable;
-    s.segments = r.eff_segments;
+    // TaskSummary.segments is minimum:1, always -- even a task that has never connected
+    // reports the count it WOULD use (its requested value, or the frozen default),
+    // never the "not started yet" placeholder of 0 that used to leak onto the wire.
+    s.segments = r.eff_segments > 0 ? r.eff_segments
+               : r.req_segments && *r.req_segments > 0 ? *r.req_segments
+               : 8;
+    if (s.segments < 1) s.segments = 1;
+    if (s.segments > 32) s.segments = 32;
     s.categoryId = r.category_id;
     s.queueId = r.queue_id;
     s.queuePosition = r.queue_position;

daemon/src/store/tasks.hpp

@@ -39,6 +39,7 @@ struct TaskRow {
 
     std::optional<std::int64_t> size_bytes;
     std::int64_t downloaded_bytes = 0;
+    std::int64_t speed_bps = 0;
     bool resumable = false;
 
     std::optional<std::int64_t> req_segments;
@@ -81,7 +82,26 @@ public:
     // Byte-counter update from an engine progress tick — cheaper than a full row rewrite,
     // and keeps download.list / download.get current between state transitions.
     DbResult<bool> update_progress(std::string_view task_id, std::int64_t downloaded_bytes,
-                                   std::int64_t eff_segments, std::int64_t eff_buffer_bytes);
+                                   std::int64_t speed_bps, std::int64_t eff_segments,
+                                   std::int64_t eff_buffer_bytes);
+
+    // What the probe learned, persisted before start() so a task that completes before any
+    // progress tick still reports a real sizeBytes / resumable (not the pre-probe default).
+    struct ProbeFields {
+        std::optional<std::int64_t> size_bytes;
+        bool resumable = false;
+        std::optional<std::string> etag;
+        std::optional<std::string> last_modified;
+        std::optional<std::string> content_type;
+        std::optional<std::string> effective_url;
+    };
+    DbResult<bool> set_probe_result(std::string_view task_id, const ProbeFields& fields);
+
+    // on_finished's byte count, for a task that completes before any progress tick ever
+    // ran (see AGENT-DAEMON review: the bug this closes). size_bytes is only filled in if
+    // still unset — the probe's total_size is the more authoritative source when both
+    // exist and happen to disagree (a chunked source with no declared length, say).
+    DbResult<bool> set_final_bytes(std::string_view task_id, std::int64_t bytes);
 
 private:
     Db& db_;