feat(trajectory): profile-scoped state embeddings (#179) - #182
Conversation
…state_id) identity (#179) Schema migration (transactional, idempotent by PK detection): legacy rows adopt the active profile's digest; sole stored digest adopted when unambiguous; mixed digests with no active profile FAIL CLOSED (overwritten vectors cannot be reconstructed — never assign mixed vectors). Restores keep-old-active-during-staging: the prior profile stays active AND intact until atomic cutover (the r5 revert existed because the old schema forbade exactly this). Pre-registered kill-bar regressions: post-migration prior-profile rankings byte-identical; interruption at any batch boundary leaves the prior profile serving identical results. Default-off feature (state_semantic_quota=0). Author: codex sol-high; review: orchestrator.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Cache: Disabled due to Reviews > Disable Cache setting Disabled knowledge base sources:
📝 WalkthroughWalkthroughState semantic embeddings are migrated to profile-scoped composite identity. Rebuilds stage vectors without deactivating the serving profile, reject unsafe same-profile rewrites, cut over only after completion, and add migration, interruption, and concurrency regression coverage. ChangesState semantic rebuild availability
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant RebuildCaller
participant TrajectoryStore
participant EmbeddingProvider
participant StateEmbeddings
participant SemanticProfiles
RebuildCaller->>TrajectoryStore: request staged rebuild
TrajectoryStore->>EmbeddingProvider: embed state documents
EmbeddingProvider-->>TrajectoryStore: return vectors
TrajectoryStore->>StateEmbeddings: upsert profile-scoped rows
TrajectoryStore->>SemanticProfiles: activate completed target
SemanticProfiles-->>RebuildCaller: expose new serving profile
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #182 - feat(trajectory): profile-scoped state embeddings (#179) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #182 Review URL: #182 (review) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95d20d0384
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_trajectory_state_semantic_expansion.py`:
- Around line 209-259: The helper
_downgrade_state_embeddings_to_legacy_primary_key uses the production migration
staging table name, creating coupling after partial failures. Rename its
temporary table and all corresponding ALTER, INSERT, and DROP references to a
test-local legacy-stage name, while leaving
_migrate_state_semantic_embeddings_to_profile_scope’s staging table unchanged.
In `@trajectory_store.py`:
- Around line 1574-1627: Validate legacy stored_digests against adopted_digest
before migrating in the profile-scoped table. In the migration logic around
adopted_digest and the INSERT into
lcm_trajectory_state_embeddings_profile_scoped, raise TrajectoryStoreError when
any stored digest differs from the adopted digest; otherwise preserve the
existing migration only for rows belonging to the adopted profile.
- Around line 1816-1834: Move the in-place rebuild guard based on
active_state_semantic_profile() and profile_digest inside the self._lock
transaction, after BEGIN IMMEDIATE, so it re-reads the active profile while
staging is protected. Preserve the existing TrajectoryStoreError condition and
message, and ensure the transaction is rolled back or otherwise safely exited
when the guard rejects the rebuild.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 885f1b09-6f44-4ed8-85f5-688bfbd5bf85
📒 Files selected for processing (2)
tests/test_trajectory_state_semantic_expansion.pytrajectory_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.14)
- GitHub Check: test (3.13)
🧰 Additional context used
🪛 ast-grep (0.45.0)
tests/test_trajectory_state_semantic_expansion.py
[info] 181-185: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
[hit.to_dict() for hit in hits],
sort_keys=True,
separators=(",", ":"),
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (7)
tests/test_trajectory_state_semantic_expansion.py (4)
180-186: Theuse-jsonifyhint is a false positive — this is a deterministic in-test digest, not an HTTP response body.Source: Linters/SAST tools
18-18: LGTM!Also applies to: 31-31, 189-206
572-606: LGTM!Also applies to: 609-625, 628-677
680-758: LGTM!Also applies to: 761-801
trajectory_store.py (3)
1480-1535: LGTM!
1733-1743: LGTM!
1923-1923: LGTM!
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #182 - feat(trajectory): profile-scoped state embeddings (#179)
Head: 95d20d0384db203a3a673b6890bce6939b4f829c into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 2/5 (~24 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
tests/test_trajectory_state_semantic_expansion.py |
modified | +219/-28 | Test coverage | Elevated: large change |
trajectory_store.py |
modified | +170/-43 | Changed file | Elevated: large change |
Review Signal
No validated inline findings.
Dropped findings before posting: 1. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
No required validation recommendation selected; rely on existing GitHub checks and human review.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Profile validation hints: Prefer correctness, security, data-loss, release, and regression findings over style-only feedback.
Profile proof expectations: Look for focused validation, rollback notes, and evidence appropriate to the changed surface.
Related Context
Related issues/PRs: #179.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: none
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
…h the active profile (refuses otherwise); TOCTOU lock ordering; recovery path documented; test staging-table isolation
|
Bot findings dispositioned (verdicts in |
evaOS review status: completedPR: #182 - feat(trajectory): profile-scoped state embeddings (#179) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #182 Review URL: #182 (review) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a9259b5f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Walkthrough
PR: #182 - feat(trajectory): profile-scoped state embeddings (#179)
Head: 5a9259b5f53f04bd91b747c8be9bf420aec30b7c into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 3/5 (~36 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
FINDINGS-VERDICTS-BOTS.md |
added | +86/-0 | Documentation | Low |
tests/test_trajectory_state_semantic_expansion.py |
modified | +284/-27 | Test coverage | Moderate: validated P3 finding |
trajectory_store.py |
modified | +180/-43 | Changed file | Elevated: large change |
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 0, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Proof gap: 1
Validation and Proof
No required validation recommendation selected; rely on existing GitHub checks and human review.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Profile validation hints: Prefer correctness, security, data-loss, release, and regression findings over style-only feedback.
Profile proof expectations: Look for focused validation, rollback notes, and evidence appropriate to the changed surface.
Related Context
Related issues/PRs: #179.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: none
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@FINDINGS-VERDICTS-BOTS.md`:
- Around line 80-81: Replace the inaccessible absolute Evidence directory path
in FINDINGS-VERDICTS-BOTS.md with artifacts stored in an accessible CI location,
or document reproducible commands that regenerate them. Ensure reviewers and CI
can access or reproduce the validation evidence without relying on the local
/Volumes path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26fc7d14-fed4-4635-9ff0-7ab611b2675b
📒 Files selected for processing (3)
FINDINGS-VERDICTS-BOTS.mdtests/test_trajectory_state_semantic_expansion.pytrajectory_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.14)
🧰 Additional context used
🪛 LanguageTool
FINDINGS-VERDICTS-BOTS.md
[grammar] ~42-~42: Ensure spelling is correct
Context: ...primary-key table. ## 3. Rebuild-guard TOCTOU - Source: CodeRabbit discussion `367328318...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (2)
trajectory_store.py (1)
1480-1535: LGTM!Also applies to: 1540-1544, 1597-1606, 1829-1839, 1933-1936
tests/test_trajectory_state_semantic_expansion.py (1)
209-259: LGTM!Also applies to: 664-724, 808-867
|
@coderabbitai review |
✅ Action performedReview finished.
|
…e; single-statement atomic cutover; positive distinct-profile resume=False coverage; round-2 verdicts
|
@coderabbitai review |
✅ Action performedReview finished.
|
evaOS review status: completedPR: #182 - feat(trajectory): profile-scoped state embeddings (#179) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #182 Review URL: #182 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #182 - feat(trajectory): profile-scoped state embeddings (#179)
Head: 367e9b49735f3442d6bb009c94e386ea3ede5b16 into main. Review event: COMMENT.
Provider: Unregistered provider id (builtin:zai-coding-plan, zcode (registry miss), model unknown).
Estimated review effort: 4/5 (~46 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
FINDINGS-VERDICTS-BOTS.md |
added | +127/-0 | Documentation | Low |
tests/test_trajectory_state_semantic_expansion.py |
modified | +388/-28 | Test coverage | Elevated: large change |
trajectory_store.py |
modified | +229/-50 | Changed file | Elevated: large change |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
No required validation recommendation selected; rely on existing GitHub checks and human review.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Profile validation hints: Prefer correctness, security, data-loss, release, and regression findings over style-only feedback.
Profile proof expectations: Look for focused validation, rollback notes, and evidence appropriate to the changed surface.
Related Context
Related issues/PRs: #179.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: none
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 367e9b4973
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@FINDINGS-VERDICTS-BOTS.md`:
- Around line 120-124: Update the validation environment to generate or provide
the missing agent.context_engine stub so the local full-suite CI-replica command
can complete test collection. Re-run the full 2711-test replica against the
current Round 2 head and update the report with its results, replacing reliance
on the narrower fallback as final sign-off.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63527104-38b9-460d-950d-8f5bded9f52c
📒 Files selected for processing (3)
FINDINGS-VERDICTS-BOTS.mdtests/test_trajectory_state_semantic_expansion.pytrajectory_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test (3.14)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
🔇 Additional comments (6)
trajectory_store.py (1)
1480-1535: LGTM!Also applies to: 1537-1644, 1806-1835, 1859-1900, 1960-1966, 2069-2087
tests/test_trajectory_state_semantic_expansion.py (3)
837-837: LGTM!Also applies to: 863-863
1036-1038: LGTM!
907-928: 🎯 Functional CorrectnessNo issue:
observed_staged_counts[0] == 0is a valid assertion._emit_progress()runs before_persist(), and the stale rows are deleted and committed before the batch loop starts.> Likely an incorrect or invalid review comment.FINDINGS-VERDICTS-BOTS.md (2)
80-95: LGTM!
96-119: LGTM!
…le is active; matrix readers synchronize across legacy-table replacement; deterministic interleaving regressions
|
@coderabbitai review |
✅ Action performedReview finished.
|
evaOS review status: completedPR: #182 - feat(trajectory): profile-scoped state embeddings (#179) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #182 Review URL: #182 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #182 - feat(trajectory): profile-scoped state embeddings (#179)
Head: 97507904cc9b14e7816b9ddba00d4ed63728fab5 into main. Review event: COMMENT.
Provider: Unregistered provider id (builtin:zai-coding-plan, zcode (registry miss), model unknown).
Estimated review effort: 5/5 (~56 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
FINDINGS-VERDICTS-BOTS.md |
added | +155/-0 | Documentation | Low |
tests/test_trajectory_state_semantic_expansion.py |
modified | +563/-28 | Test coverage | Elevated: large change |
trajectory_store.py |
modified | +267/-74 | Changed file | Elevated: large change |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
No required validation recommendation selected; rely on existing GitHub checks and human review.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Profile validation hints: Prefer correctness, security, data-loss, release, and regression findings over style-only feedback.
Profile proof expectations: Look for focused validation, rollback notes, and evidence appropriate to the changed surface.
Related Context
Related issues/PRs: #179.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: none
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97507904cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… available; leasing hardening deferred to issue
evaOS review status: completedPR: #182 - feat(trajectory): profile-scoped state embeddings (#179) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #182 Review URL: #182 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #182 - feat(trajectory): profile-scoped state embeddings (#179)
Head: 7af390c25279c10231e3686c18d402c65fb1a8df into main. Review event: COMMENT.
Provider: Unregistered provider id (builtin:zai-coding-plan, zcode (registry miss), model unknown).
Estimated review effort: 5/5 (~56 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
FINDINGS-VERDICTS-BOTS.md |
added | +162/-0 | Documentation | Low |
tests/test_trajectory_state_semantic_expansion.py |
modified | +563/-28 | Test coverage | Elevated: large change |
trajectory_store.py |
modified | +269/-74 | Changed file | Elevated: large change |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
No required validation recommendation selected; rely on existing GitHub checks and human review.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Profile validation hints: Prefer correctness, security, data-loss, release, and regression findings over style-only feedback.
Profile proof expectations: Look for focused validation, rollback notes, and evidence appropriate to the changed surface.
Related Context
Related issues/PRs: #179.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: none
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Review-round stopping rule for this train (declared per program rule — red-team amendment 9 pattern): round 5 on the new head is the FINAL full round. From round 5 onward, findings are dispositioned as: real delivery-affecting or P1-severity defects → fixed in-PR; latent hardening, future-proofing, and documentation-class items → fork issues for the next train (as #186 was). This PR is default-off infrastructure; its gate-run obligations (none — no measured surface) are unchanged. All prior rounds: every finding fixed, refuted in writing, or issue-filed, with threads resolved. |
…d (root filename collision with #184's record)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff52c487b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._conn.execute( | ||
| "UPDATE lcm_trajectory_state_embedding_profiles " | ||
| "SET active = 0 WHERE active = 1" | ||
| ) | ||
| self._conn.execute( | ||
| "UPDATE lcm_trajectory_state_embedding_profiles " | ||
| "SET active = 1 WHERE profile_digest = ?", | ||
| (profile_digest,), | ||
| """ | ||
| INSERT INTO lcm_trajectory_state_embedding_profiles( |
There was a problem hiding this comment.
When a concurrent resume=False build deletes this inactive target's staged rows after embedded_count is read but before this transaction begins, this statement still activates the now-empty or partial profile because the cutover never rechecks completeness under BEGIN IMMEDIATE. The other builder is then rejected by _persist because the target is active, leaving an incomplete serving profile that cannot be resumed through this API; repeat the row-count validation inside the cutover transaction before setting active = 1.
Useful? React with 👍 / 👎.
The branch had merged the MAINTAINER's upstream (v0.21.0-rc2) and that was mistaken for being current. It was not current with OUR main, which carried 56 commits this branch had never seen: V1-M instrument (#199, #198), fts_prose_mode (#183), profile-scoped trajectory embeddings (#182), query views. Bidirectional, not a catch-up -- both lines developed the same files in parallel (tools.py ours=11/theirs=9, vector_store.py 6/8, store.py 5/5), and reasoning.py and requirements_compiler.py do not exist at the merge base at all. 15 files, 52 hunks. Default resolution was BOTH SIDES: ours' security parameter and theirs' feature parameter, additively. ## The leak that was in no conflict hunk Main added a resident-int8 full-corpus fast path (`resident_eligible` -> `_resident_int8_matrix` -> `_rank_resident_int8`) that sits immediately BEFORE the sign-bit prescreen block ours guards with `access_scope is None`, and mirrors that block's eligibility conditions -- without the owner guard. Git auto-merged it CLEANLY. Our guard survived while a bypass around it landed one branch earlier, with no conflict marker anywhere and nothing for a diff review to catch. The resident matrix is a whole-identity snapshot pooled and cached on (identity, data_version) only, so it cannot express a per-row owner predicate. Filtering inside it is not an option; routing around it is, exactly as ours already does for the binary prescreen. Both arms now carry `access_scope is None`. ## The trap that WAS in a conflict hunk Main refactored the message-search call to `**message_search_kwargs`, and that dict carried every field ours listed EXCEPT `access_scope`. Taking main's side verbatim silently unscopes message search -- every principal reads every other principal's memory, with all tests green. Carried into the new form. ## Reconciling two independent models of reasoning.py `resolve_occurrence_time_with_trust(session_date=...)` served two roles at once: theirs' caller-DECLARED anchor (the term the sidecar is compared against to detect disagreement) and the term the occurrence is RESOLVED against. Ours needs the resolution anchor to be host-derived and a caller's declaration to be evidence-free. No single ordering satisfies both: map-first defeats the override check entirely, because tools.py sources that map from `engine._session_occurrence_dates` -- the sidecar's own attribute -- so anchor and sidecar were always equal; declared-first lets a caller relabel a stored observation. Split into two parameters, precedence sidecar > trusted > declared. Main's `"auto"` interval-unit sentinel is replaced by `interval_unit_fixed`, so "the plan's unit" and "the reported unit" decouple and both sides' tests hold. ## Verified, not asserted real-data battery 16/16, identical to the pre-merge baseline default-off A/B 22/22 byte-identical to stock origin/main compile 224 files, 0 syntax errors suite 21 failed / 3234 passed (from 35/3008 pre-merge) The 4b gate also did its job on this merge: it failed closed on six new resident-invalidation triggers and on a `benchmarking/` harness the exclusion list had missed (the -ing spelling was never added alongside bench/benchmarks). Both classified rather than waived.
1 participant
Closes #179. Schema migration to
(profile_digest, state_id)identity with fail-closed handling of ambiguous legacy state, restoring rebuild availability (old profile serves, intact, until atomic cutover — the sound version of what round 4 attempted and round 5 had to revert).Kill bars pre-registered in the spec and asserted as regressions: post-migration ranking byte-identity; interruption-at-any-boundary leaves the prior profile serving identical results. Default-off feature; full-suite exact-name parity in the CI-replica env.
Review per program protocol — bots, findings get fixed/refuted/deferred with verdicts.
Summary by CodeRabbit
New Features
Bug Fixes