Skip to content

non unique TS in sqlite#2342

Open
leshy wants to merge 3 commits into
mainfrom
fix/ivan/mem2timestamps
Open

non unique TS in sqlite#2342
leshy wants to merge 3 commits into
mainfrom
fix/ivan/mem2timestamps

Conversation

@leshy
Copy link
Copy Markdown
Member

@leshy leshy commented Jun 3, 2026

we gave up on sensor assigned timestamps on go2

(timestamps have different weird bugs on different firmware versions, unable to get image timestamps from the device)

so now we use time.time()

issue is that during network dropout issues and latency, we receive a bunch of messages together, and hit UNIQUE constraints in mem2 for ts.. for now unblocking this by allowing non UNIQUE ts storage, and will investigate solutions later

PS also removing markers detector from memory blueprint

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 3, 2026

Greptile Summary

This PR addresses duplicate-timestamp UNIQUE constraint failures in the SQLite observation store that occur when time.time() is used for timestamps and messages arrive in bursts (e.g. during network dropout recovery). It also simplifies the unitree_go2_memory blueprint by connecting directly to unitree_go2 rather than the marker-augmented pipeline, since Go2Memory never consumed marker outputs.

  • sqlite.py: Removes UNIQUE from the ts column definition so new databases accept duplicate timestamps. Existing databases are unaffected because CREATE TABLE IF NOT EXISTS skips re-creation.
  • unitree_go2.py: Swaps unitree_go2_markers for unitree_go2 as the upstream for unitree_go2_memory; functionally equivalent for Go2Memory since it only subscribes to color_image, lidar, and odom.

Confidence Score: 4/5

Safe to merge for fresh deployments, but existing databases with the old UNIQUE constraint on ts will not benefit from the fix and will continue to throw constraint errors on duplicate timestamps.

The schema change correctly removes the UNIQUE constraint for new databases, but _ensure_tables uses CREATE TABLE IF NOT EXISTS, so any on-disk database already carrying the old constraint is untouched. Deployments with existing store files will still hit the exact failure the PR means to resolve.

dimos/memory2/observationstore/sqlite.py — the _ensure_tables method needs a migration path to cover existing databases.

Important Files Changed

Filename Overview
dimos/memory2/observationstore/sqlite.py Drops UNIQUE constraint from ts column, but uses CREATE TABLE IF NOT EXISTS — existing databases keep the old unique constraint and will still fail on duplicate timestamps.
dimos/robot/unitree/go2/blueprints/smart/unitree_go2.py Rewires unitree_go2_memory to connect directly to unitree_go2 instead of unitree_go2_markers; Go2Memory only consumes color_image, lidar, and odom so marker-pipeline outputs were never used in this context.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["time.time() timestamps\n(may duplicate on burst)"] --> B["Go2Memory.insert(obs)"]
    B --> C{"Table exists?"}
    C -- "New DB\n(no UNIQUE)" --> D["INSERT succeeds ✓"]
    C -- "Old DB\n(UNIQUE constraint)" --> E["UNIQUE constraint failed ✗"]
    E --> F["Exception — observation dropped"]
    D --> G["SQLite: ts REAL NOT NULL"]
Loading

Comments Outside Diff (1)

  1. dimos/memory2/observationstore/sqlite.py, line 256-267 (link)

    P1 Schema change silently ignored on existing databases

    CREATE TABLE IF NOT EXISTS only creates the table when it does not already exist. Any recording_go2.db (or other store) created by a prior build still has ts REAL NOT NULL UNIQUE, and SQLite provides no ALTER TABLE ... DROP CONSTRAINT command. Those databases will continue to raise UNIQUE constraint failed on every burst of same-timestamp inserts — the exact problem this PR intends to fix. A one-time migration (e.g. PRAGMA table_info(...) to detect the old schema, then recreate the table and copy rows) is needed before any deployment with an existing database will benefit from this change.

Reviews (2): Last reviewed commit: "Merge branch 'fix/ivan/mem2timestamps' o..." | Re-trigger Greptile

Comment thread dimos/robot/unitree/go2/blueprints/smart/unitree_go2.py
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

paul-nechifor
paul-nechifor previously approved these changes Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants