Skip to content

feat: Phase 4 — Gym & Benchmark integration via amplihack-agent-eval bridge - #108

Merged
rysweet merged 1 commit into
mainfrom
feat/phase-4-gym-eval
Mar 31, 2026
Merged

feat: Phase 4 — Gym & Benchmark integration via amplihack-agent-eval bridge#108
rysweet merged 1 commit into
mainfrom
feat/phase-4-gym-eval

Conversation

@rysweet

@rysweet rysweet commented Mar 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add GymBridge (Rust) wrapping BridgeTransport to call the Python gym eval server with typed RPC methods: list_scenarios, run_scenario, run_suite
  • Add gym_scoring module with aggregate_suite_scores, detect_regression, and track_improvement operating on five scoring dimensions (factual_accuracy, specificity, temporal_awareness, source_attribution, confidence_calibration)
  • Add simard_gym_bridge.py extending BridgeServer to wrap amplihack-agent-eval's progressive test suite (L1-L12) and long-horizon memory evaluation
  • Integration tests validate the full pipeline: bridge call -> deserialization -> scoring -> regression detection -> improvement trend tracking

Design decisions

  • Five scoring dimensions align with long_horizon_memory.ALL_DIMENSIONS from amplihack-agent-eval
  • Python bridge imports progressive suite and long-horizon eval lazily; missing components are logged as degraded sources (Pillar 11)
  • Regression detection uses a 0.01 threshold with Minor/Moderate/Severe severity classification
  • Improvement tracking requires >= 2 data points; single entry returns Stable

File sizes

File LOC
src/gym_bridge.rs 300
src/gym_scoring.rs 299
python/simard_gym_bridge.py 282
tests/gym_eval.rs 374

Test plan

  • cargo test — all 128 unit tests + 70 integration tests + 13 gym_eval tests pass
  • cargo clippy -- -D warnings — clean
  • cargo fmt -- --check — clean
  • Manual: verify simard_gym_bridge.py runs against live amplihack-agent-eval (requires eval package installed)

Closes #93

🤖 Generated with Claude Code

Simard Test @claude
feat: Phase 4 — Gym & Benchmark integration via amplihack-agent-eval …
33eaefa 
…bridge

Add GymBridge wrapping BridgeTransport to drive amplihack-agent-eval's
progressive test suite (L1-L12) and long-horizon memory evaluations from
Rust. Score aggregation, regression detection, and improvement trend
tracking operate on typed result structs with five scoring dimensions
(factual_accuracy, specificity, temporal_awareness, source_attribution,
confidence_calibration). Python bridge server extends BridgeServer to
wrap the eval harness. All failures surface with full context per
Pillar 11 (honest degradation).

Closes #93

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rysweet
rysweet merged commit e30417f into main Mar 31, 2026
3 checks passed
This was referenced Mar 31, 2026
Closed
Closed
Closed
@rysweet rysweet mentioned this pull request May 11, 2026
Open
@rysweet rysweet mentioned this pull request Jun 24, 2026
Merged
rysweet pushed a commit that referenced this pull request Jun 24, 2026
Simard Engineer
Merge origin/main into #107 re-pin; resolve to prebuilt lbug 0.17.1 + #…
e6dbc2f 
…108 fix

Resolves the Cargo.toml/Cargo.lock conflict with the reverted main (4e01fbb):
- amplihack-memory re-pinned to the squash-merge SHA of amplihack-memory-lib
  PR #108 (26d49bf — the #107 _deleted read-path fix + empty-read safety gate).
- lbug = =0.17.1 (prebuilt crates.io engine; NO [patch.crates-io]). The CSR
  SIGSEGV defense is the lib's tombstone soft-delete, so the from-source C++
  patch is intentionally dropped (faster builds; prebuilt 0.17.1 reads v40 fine).
- Single lbug 0.17.1 in the graph; 0 lbug-patched refs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rysweet added a commit that referenced this pull request Jun 24, 2026
@rysweet Simard Engineer
feat(safe-update): re-pin lbug 0.15.3→0.17.1 (#107-fixed amplihack-me…
bf59da6 
…mory) + safe-update item-count parity gate (#2400)

* feat(safe-update): re-pin lbug 0.15.3 -> 0.17.1 (#107-fixed amplihack-memory) + item-count parity gate

Re-enables the lbug 0.15.4 -> 0.17.1 engine migration, now that the
amplihack-memory-lib #107 "silent empty-read" data-loss bug is fixed.

Re-pin (Cargo.toml / Cargo.lock):
- Bump the direct `lbug` dep (used by simard-tui's read-only goal-board view)
  from "=0.15.3" to "=0.17.1", in lockstep with the amplihack-memory pin so the
  final binary links exactly ONE engine and ONE storage format (v41). Verified
  with `cargo tree -i lbug` => a single `lbug v0.17.1` (direct + transitive).
- Re-pin the `amplihack-memory` git dependency to the commit carrying the
  0.15.4->0.17.1 engine migration AND the #107 fix (the `_deleted` tombstone
  read-path fix + the empty-read safety gate). Repoint to the squash-merge
  commit on amplihack-memory-lib `main` once PR #108 merges.

Safe-update item-count parity gate (defense-in-depth, the doc's "second,
independent defense layer"):
- The validate phase previously declared an upgrade `validated` after N clean
  OODA cycles ONLY. A silently-empty store passes clean cycles, so that alone is
  not evidence of a healthy upgrade. Now `record_cycle_with_parity` compares the
  incoming binary's cognitive item count against a pre-upgrade baseline recorded
  in `upgrade-status.json`; on a shortfall (the #107 signature: pre > 0, post
  read back smaller / empty) it forces `validate_timeout` instead of `validated`,
  so the watchdog rolls the upgrade back. `record_cycle` delegates with no
  baseline, preserving existing behaviour.
- `UpgradeStatus.pre_upgrade_item_count` (serde default) carries the baseline;
  `UpdateConfig.pre_upgrade_item_count` threads it through the orchestrator ->
  `do_swap` -> `exec_handover`. The operator CLI leaves it `None` (it must NOT
  open the live store); the brain sets it in-process from its already-open store.
- Pure `memory_parity_verdict(pre, current) -> ParityVerdict` + unit tests.

The hard, always-on guarantee is the amplihack-memory store-open gate (which the
re-pinned daemon now links); this Simard gate is the second layer. Brain-loop
wiring of the actual counts (record pre-upgrade count at update initiation; pass
the current count to `record_cycle_with_parity` at validate) requires the live
daemon's open store and is the remaining live-deploy step.

Does NOT deploy 0.17.1 to the live daemon. All 55 safe_update + 60
cognitive_memory tests pass; clippy + fmt clean.

Refs amplihack-memory-lib#107
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore(deps): bump quinn-proto 0.11.14 -> 0.11.15 (RUSTSEC-2026-0185 memory exhaustion)

cargo-audit flagged RUSTSEC-2026-0185 (high; remote memory exhaustion in
quinn-proto's out-of-order stream reassembly) — a newly-published advisory whose
fix is >=0.11.15. A transitive dep; the precise bump resolves it with no other
lockfile churn.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(logging): route tracing logs to stderr so --json command stdout stays clean

The lbug 0.17.1 / newer amplihack-memory rev emits an INFO log on store-open;
with the tracing fmt layer writing to STDOUT, that log interleaved with the JSON
emitted by `simard memory stats|dump --json`, breaking the 3
bin_simard_memory_cli integration tests (stdout no longer parseable as JSON).

Route the fmt layer to stderr (standard CLI hygiene: logs -> stderr, results ->
stdout). Fixes the pollution for every --json command, not just memory stats.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Simard Engineer <engineer@simard.local>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rysweet pushed a commit that referenced this pull request Jun 28, 2026
Simard Engineer
fix(memory): verified backups of the LIVE cognitive store + bounded q…
33c7f58 
…uarantines (#2420)

Verified cognitive-memory backups silently stopped on Jun 20: the backup
copied the legacy single-file store `state_root/cognitive_memory.ladybug`,
but after the lbug-0.17.x de-fork migration (#2307) the live store the daemon
opens is `state_root/cognitive`. Two coupled defects are fixed.

1. Backup source == live store (migration-aware).
   `cognitive_memory::live_store_path` resolves the live store path (prefer the
   `cognitive` subdir, fall back to the legacy file only on an un-migrated host,
   default to `cognitive`). The daemon store-open and `memory_backup::backup_source_path`
   both route through it; a regression test asserts the backup source equals the
   path `LibraryCognitiveMemory::open` uses, so they can never silently diverge.

2. Whole-store export (no silent cap).
   The snapshot export capped at MAX_EXPORT_FACTS=1000, so a backup of the
   ~10.8k-memory live store was hollow. New `remote_transfer::export_full_memory_snapshot`
   captures the entire store (uncapped, same unbounded retrieval `graph_stats`
   already does); `backup_memory` uses it. A test stores >1000 facts and asserts
   the verified backup round-trips the full count on restore.

3. Verify-before-prune gate.
   `backup_memory_verified` / `ensure_backup_valid` re-open and validate a backup
   (checksum + manifest self-consistency) and fail loudly unless Valid;
   `verify_backup_memory_count` rejects a count mismatch. Pruning runs only after
   a verified backup, so a failed/partial backup can never delete the last good one.

4. Bounded corrupt-quarantine retention.
   `remove_old_corrupt_dbs` now removes a quarantine if it is older than
   CORRUPT_DB_MAX_AGE_DAYS (7) OR not among the newest CORRUPT_DB_KEEP (5),
   bounding the burst growth the age cap alone left unbounded. The live store
   files are never matched. The concatenating quarantine *rename* lives in lbug
   WAL recovery (already at the WAL-durability rev — see below), not Simard.

5. Daemon periodic verified backup.
   Reintroduces the per-cycle backup removed in #2307 as a best-effort *verified*
   routine gated by SIMARD_BACKUP_INTERVAL_SECS (default daily). The first backup
   always runs on the first cycle (`should_run_backup(None, _)`), so a freshly
   rebooted/deployed host backs up promptly instead of deferring a full interval
   (Linux `Instant` is boot-relative, so a `checked_sub` back-date would underflow
   on a young host). A failure WARNs and skips the prune; it never aborts the cycle.
   Operator deploys — the daemon is not live-redeployed here.

6. amplihack-memory WAL durability — verify-only, no bump.
   The WAL-durability commit 7b81590 (recover-from-corrupt-WAL + checkpoint, PR #89)
   is already an ancestor of the current pin 26d49bf8 (lbug-0.17.1, PR #108).
   Re-bumping would be a no-op/regression, so the pin is unchanged.

Adds unit tests plus a restore round-trip (incl. a >cap store) and a docs page
(docs/operations/verified-backups.md). Build, fmt, clippy -D warnings, and the
affected-module + CI release test subsets are green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rysweet rysweet mentioned this pull request Jun 28, 2026
Closed
rysweet added a commit that referenced this pull request Jun 28, 2026
@rysweet Simard Engineer
fix(memory): verified backups of the LIVE cognitive store + bounded q…
048f9f2 
…uarantines (#2420) (#2466)

* fix(memory): verified backups of the LIVE cognitive store + bounded quarantines (#2420)

Verified cognitive-memory backups silently stopped on Jun 20: the backup
copied the legacy single-file store `state_root/cognitive_memory.ladybug`,
but after the lbug-0.17.x de-fork migration (#2307) the live store the daemon
opens is `state_root/cognitive`. Two coupled defects are fixed.

1. Backup source == live store (migration-aware).
   `cognitive_memory::live_store_path` resolves the live store path (prefer the
   `cognitive` subdir, fall back to the legacy file only on an un-migrated host,
   default to `cognitive`). The daemon store-open and `memory_backup::backup_source_path`
   both route through it; a regression test asserts the backup source equals the
   path `LibraryCognitiveMemory::open` uses, so they can never silently diverge.

2. Whole-store export (no silent cap).
   The snapshot export capped at MAX_EXPORT_FACTS=1000, so a backup of the
   ~10.8k-memory live store was hollow. New `remote_transfer::export_full_memory_snapshot`
   captures the entire store (uncapped, same unbounded retrieval `graph_stats`
   already does); `backup_memory` uses it. A test stores >1000 facts and asserts
   the verified backup round-trips the full count on restore.

3. Verify-before-prune gate.
   `backup_memory_verified` / `ensure_backup_valid` re-open and validate a backup
   (checksum + manifest self-consistency) and fail loudly unless Valid;
   `verify_backup_memory_count` rejects a count mismatch. Pruning runs only after
   a verified backup, so a failed/partial backup can never delete the last good one.

4. Bounded corrupt-quarantine retention.
   `remove_old_corrupt_dbs` now removes a quarantine if it is older than
   CORRUPT_DB_MAX_AGE_DAYS (7) OR not among the newest CORRUPT_DB_KEEP (5),
   bounding the burst growth the age cap alone left unbounded. The live store
   files are never matched. The concatenating quarantine *rename* lives in lbug
   WAL recovery (already at the WAL-durability rev — see below), not Simard.

5. Daemon periodic verified backup.
   Reintroduces the per-cycle backup removed in #2307 as a best-effort *verified*
   routine gated by SIMARD_BACKUP_INTERVAL_SECS (default daily). The first backup
   always runs on the first cycle (`should_run_backup(None, _)`), so a freshly
   rebooted/deployed host backs up promptly instead of deferring a full interval
   (Linux `Instant` is boot-relative, so a `checked_sub` back-date would underflow
   on a young host). A failure WARNs and skips the prune; it never aborts the cycle.
   Operator deploys — the daemon is not live-redeployed here.

6. amplihack-memory WAL durability — verify-only, no bump.
   The WAL-durability commit 7b81590 (recover-from-corrupt-WAL + checkpoint, PR #89)
   is already an ancestor of the current pin 26d49bf8 (lbug-0.17.1, PR #108).
   Re-bumping would be a no-op/regression, so the pin is unchanged.

Adds unit tests plus a restore round-trip (incl. a >cap store) and a docs page
(docs/operations/verified-backups.md). Build, fmt, clippy -D warnings, and the
affected-module + CI release test subsets are green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* refactor(memory): anchor backup paths to single-source constants (#2420)

Review-pass cleanup of the verified-backup fix. Removes the last places where
the same path/filename was spelled twice and could silently drift.

- `LibraryCognitiveMemory::open` now joins `LIVE_STORE_SUBDIR` instead of the
  bare "cognitive" literal, so the daemon store-open and the `live_store_path`
  resolver are anchored to one constant by construction.
- Drop the redundant `memory_backup::backup_source_path` delegate (a thin
  wrapper over `cognitive_memory::live_store_path`) and its standalone test.
  The stronger "backup source == the path the daemon actually opens" guarantee
  is pinned by `cognitive_memory::tests_live_store_path_2420::matches_daemon_open_path`,
  which opens the store via the daemon path and asserts equality.
- `daemon::backup` imports the canonical `MEMORY_RECORDS_FILENAME` from
  `meeting_backend::persist` (same "memory_records.json" value) instead of
  re-declaring it, so the backup and the writer cannot disagree.
- Docs updated to match (LIVE_STORE_SUBDIR anchoring; backup_source_path removed).

No behavior change. Verified: cargo check, rustfmt, clippy clean; 27 targeted
tests pass (tests_live_store_path_2420, memory_backup, daemon::backup).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(memory): read backup files from backup_dir, not untrusted manifest paths (#2420)

Security review (High): `verify_backup` and `restore_from_backup` read the
snapshot/records files from the absolute paths stored inside `manifest.json`
(`cognitive_snapshot_path` / `memory_records_path`). The manifest is untrusted
on-disk data, so a tampered manifest could redirect those reads to arbitrary
files outside the backup directory — a path-traversal / arbitrary-file-read
vector that, on restore, ingests external data into the live cognitive store.
It also broke relocated backups (stale absolute paths).

Fix: derive the data-file paths from the `backup_dir` argument plus the known
constant filenames (SNAPSHOT_FILE / RECORDS_FILE), exactly how `backup_memory`
writes them — never from the manifest. The manifest path fields are now
documented as informational only. Legitimate backups are bit-identical, so
checksum/count verification is unchanged.

Tests:
- verify_and_restore_use_backup_dir_not_manifest_paths: a relocated backup
  (manifest holds stale absolute paths) still verifies Valid and restores.
- restore_does_not_read_manifest_paths_outside_backup_dir: a manifest crafted
  to point at attacker files outside the backup dir never injects that data.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(memory): real on-disk e2e verified-backup round-trip of the LIVE store (#2420)

Outside-in acceptance guard mirroring the OODA daemon's production flow against
a REAL persistent lbug store (existing daemon tests use an in-memory bridge):
open the live `state_root/cognitive` store, assert backup-source == live store,
populate >1000 memories (past the legacy export cap), take a verified backup,
count-verify it before pruning, then restore into a fresh store and assert the
full memory count round-trips.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Simard Engineer <engineer@simard.local>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: Phase 4 — Gym & benchmark integration via amplihack-agent-eval

1 participant

@rysweet