Skip to content

fix: triage batch — Windows compat, preflight wiring, schema FLEXIBLE (v0.12.0)#92

Closed
Knapp-Kevin wants to merge 1 commit into
BicameralAI:devfrom
Knapp-Kevin:fix/triage-batch-pr-a
Closed

fix: triage batch — Windows compat, preflight wiring, schema FLEXIBLE (v0.12.0)#92
Knapp-Kevin wants to merge 1 commit into
BicameralAI:devfrom
Knapp-Kevin:fix/triage-batch-pr-a

Conversation

@Knapp-Kevin

Copy link
Copy Markdown
Collaborator

Summary

Triage batch fixing four P0/P1 issues that block Windows development and a
schema-level data-loss bug. All four were planned together via QorLogic
SDLC (/qor-plan → 3 audit rounds → /qor-implement/qor-substantiate).

Issue Fix
#74 Platform-gate import fcntl + threading.Lock for Windows safety. Test suite now collects on Windows.
#69 Implement _merge_decision_matches and wire keyword-match path into handle_preflight. Status-aware fired gating.
#68 Normalize Windows paths in surrealkv:// URLs so urllib/SDK accepts them.
#72 Add FLEXIBLE to binds_to.provenance. Schema v11→v12 migration with legacy stamp.

Schema migration note (#72)

  • Version: SCHEMA_VERSION 11 → 12.
  • Direction: forward-only. Once a DB is migrated to v12, older binaries will refuse to start (existing SchemaVersionTooNew UX).
  • Destructive?: No. Migration uses DEFINE FIELD OVERWRITE to redefine the existing column with FLEXIBLE and runs an UPDATE that stamps pre-existing rows (those with provenance = {} or NONE) with {"_pre_schema_v12": true}. No data is lost; the stamp lets consumers distinguish "lost provenance at write time" from "genuinely empty".
  • Idempotent: Yes. The orchestrator at migrate() short-circuits when current == SCHEMA_VERSION.

Behavior change (#69)

bicameral.preflight now performs the keyword-match step its docstring documented but never executed. In normal mode, preflight will now fire=True on keyword matches whose status is drifted or ungrounded (previously: only region matches drove fired). In guided mode, any merged match fires. This is a documented broadening — see handler docstring step 8.

The keyword path calls ctx.ledger.search_by_query directly, not handle_search_decisions — the latter would trigger a nested handle_link_commit and double-sync per preflight call. The regression test test_handle_preflight_does_not_trigger_link_commit locks this contract.

Test plan

  • tests/test_events_writer_windows.py (new, 4 tests) — Windows import + concurrent-write
  • tests/test_match_shaping.py (new, 9 tests) — _raw_to_decision_match unit
  • tests/test_preflight_merge_wiring.py (new, 7 tests) — no-cascade contract + status gating
  • tests/test_v055_region_anchored_preflight.py (modified, 10 tests) — wiring updated for direct primitive
  • tests/test_surrealkv_url_windows.py (new, 4 tests) — URL probe + round-trip on Windows
  • tests/test_schema_persistence.py (modified, 5 tests) — uses helper for URL construction
  • tests/test_binds_to_provenance_flexible.py (new, 7 tests) — FLEXIBLE round-trip + migration idempotency + row-count + legacy filter
  • tests/test_phase2_ledger.py (15 tests) — regression gate for search_decisions refactor (behavior preservation)

PR-A total: 61/61 passing on Windows.

26 pre-existing failures across the broader suite are documented Windows-specific issues unrelated to this PR (#67 subprocess NotADirectoryError, cp1252 encoding, alpha_flow assertions). None touch files in this PR.

Windows verification

All four fixes were authored and verified on a Windows host:

  • Phase 1: events.writer imports cleanly on Windows; concurrent-write test passes.
  • Phase 3: _surrealkv_url_for_path round-trip probe verified live against AsyncEmbeddedDB on Windows. Five candidate URL shapes were tested; the winning shape (surrealkv://C:/Users/foo/ledger.db — two-slash, forward slashes only, no leading slash) was the only one passing both urlparse and the Rust embedded datastore.
  • All other phases run cleanly on Windows.

Out of scope (future PR-B)

The triage plan also covers two issues deferred to PR-B for focused review:

  • #39 — Local-only telemetry counters + first-boot consent notice.
  • #75 — Document decision_level in ARCHITECTURE_PLAN.md.

Closes

Closes #74
Closes #69
Closes #68
Closes #72

🤖 Authored via QorLogic SDLC (plan → audit → implement → substantiate). Substantiation seal: sha256:306b40d4625a82f4.

… (v0.12.0)

Four-issue triage batch authored via QorLogic SDLC (plan → audit ×3 → implement
→ substantiate). All four issues block development on Windows or cause silent
data loss; fixes are minimal and well-scoped.

Closes BicameralAI#74 — events/writer.py: top-level `import fcntl` was POSIX-only and
crashed test collection on Windows. Platform-gated import + cross-platform
_lock_exclusive/_unlock helpers; threading.Lock for in-process safety.

Closes BicameralAI#69 — bicameral.preflight: implemented the keyword-match step that
the docstring promised (step 5) but never executed. Preflight now calls
ctx.ledger.search_by_query directly (NOT handle_search_decisions, which
would re-trigger handle_link_commit and double-sync). New helper
handlers/_match_shaping._raw_to_decision_match factored out of
handle_search_decisions for sharing. _merge_decision_matches dedupes by
decision_id with region matches winning on collision. fired gating is now
status-aware per docstring step 8 — keyword matches with drifted/ungrounded
status now fire preflight in normal mode (documented broadening).

Closes BicameralAI#68 — ledger/adapter.py: `surrealkv://` URLs with Windows paths
raised `ValueError: Port could not be cast` because urllib interpreted the
drive-letter colon as a host:port separator. New _surrealkv_url_for_path
helper normalizes backslashes to forward slashes. Five candidate URL shapes
were probed live against AsyncEmbeddedDB on Windows; the winning shape
(surrealkv://C:/Users/foo/ledger.db — two-slash, forward slashes only, no
leading slash) was the only one accepted by both urlparse and the Rust
embedded datastore.

Closes BicameralAI#72 — ledger/schema.py: binds_to.provenance was declared `TYPE
object DEFAULT {}` without FLEXIBLE, so SurrealDB v2 silently dropped any
sub-keys at write time. Every row created since v0.5.0 has provenance={}.
Schema bumped v11→v12 with new _migrate_v11_to_v12: DEFINE FIELD OVERWRITE
to redefine non-destructively, then UPDATE stamps pre-fix rows with
{"_pre_schema_v12": true} so consumers can distinguish lost-provenance from
genuinely-empty. Underscore-prefixed keys on binds_to.provenance reserved
as system metadata.

Tests: 61/61 PR-A pass. Verified on Windows host. 26 pre-existing failures
across the broader suite are documented as BicameralAI#67 (Windows subprocess) and
unrelated encoding issues — none touch files in this PR.

Substantiation seal: sha256:306b40d4625a82f4

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46379406-be92-4754-8f57-ce1289e8c1a3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Knapp-Kevin

Copy link
Copy Markdown
Collaborator Author

Closing as redundant — all four issues were already addressed on dev via separate PRs that I missed because my branch was based on a stale fork (origin/dev was 9 commits behind upstream/dev when I started):

The independent verification work (e.g. the URL-shape probe in #68, the threading.Lock for in-process safety in #74, the OVERWRITE keyword choice in #72) confirms the upstream fixes work — but the actual code changes here would just collide with what's already merged. No action needed; closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant