feat(lineage)!: B3 Task 7 — hard-remove mentioned_profiles + retire legacy ProfileChangeLog read path - #213
Merged
Merged
Conversation
…ChangeLog read path (B3 Task 7) BREAKING CHANGE: the always-empty `mentioned_profiles` field is removed from the ProfileChangeLog / ProfileChangeLogView API + SDK response shape. Read `added_profiles` / `removed_profiles` instead. (T5 deprecated it; it has always been an empty list, and reconstruction never populated it.) - remove `mentioned_profiles` from the domain + view models, the reconstruction output, the UI converter, SDK exports + client, and the SQLite/Supabase/Postgres storage shapes (NOT the frozen `_DDL` CREATE TABLE — that is Task 8). - retire the legacy-vs-reconstruction parity machinery (reconstruction STAYS): delete `lib/_lineage_parity.py`, `scripts/lineage_b3_parity_check.py`, the Task-4 gate + classifier tests; remove the storage `get_profile_change_logs` + `add_profile_change_log` methods + their converters; remove the `RetentionTarget` for profile_change_logs. - relocate the reconstruction's storage protocol out of the parity module into `lib/_profiles.py`, renamed `ChangeLogReadStorage` (dropping `get_profile_change_logs`). - repoint the low-priority e2e tests off the removed storage read method. - set `major_on_zero = false` so this pre-1.0 breaking change releases as 0.3.0, not 1.0.0. KEEP (Task 8): the legacy `profile_change_logs` table + the `delete_all` / `delete_for_user` storage methods (the GDPR remap + table rename/drop land there).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoves the B3 lineage parity check infrastructure ( ChangesB3 parity retirement and reconstruction finalization
CI/Semantic release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
reflexio/lib/_profiles.py (1)
585-589: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim the unused per-request methods from
ChangeLogReadStorage.
reconstruct_profile_change_lognow reads additions viaget_all_generated_profiles, so keepingget_distinct_generated_from_request_idsandget_profiles_by_generated_from_request_idin this protocol makes new readers implement legacy hooks the function no longer needs.♻️ Proposed protocol cleanup
- def get_distinct_generated_from_request_ids(self) -> list[str]: ... - - def get_profiles_by_generated_from_request_id( - self, request_id: str - ) -> list[UserProfile]: ... - def get_all_generated_profiles(self) -> list[UserProfile]: ...🤖 Prompt for 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. In `@reflexio/lib/_profiles.py` around lines 585 - 589, Remove the unused method signatures get_distinct_generated_from_request_ids and get_profiles_by_generated_from_request_id from the ChangeLogReadStorage protocol. Since reconstruct_profile_change_log now uses get_all_generated_profiles for reading additions, these per-request methods are no longer needed and their removal will prevent new implementations from being forced to implement legacy hooks that are no longer utilized.tests/server/api_endpoints/test_profile_change_log_api_integration.py (1)
73-87: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert removed-field absence in the raw API payload.
To lock the v0.3.0 contract, add an explicit raw-JSON assertion that
mentioned_profilesis absent for the reconstructed row (model parsing alone can miss extra keys depending config).Suggested test hardening
resp = client.get("/api/profile_change_log") assert resp.status_code == 200, resp.text -parsed = ProfileChangeLogViewResponse(**resp.json()) +payload = resp.json() +parsed = ProfileChangeLogViewResponse(**payload) assert parsed.success is True rows = {row.request_id: row for row in parsed.profile_change_logs} assert run_id in rows, ( @@ row = rows[run_id] +raw_row = next(r for r in payload["profile_change_logs"] if r["request_id"] == run_id) +assert "mentioned_profiles" not in raw_row assert [p.profile_id for p in row.added_profiles] == ["p-new-1"]🤖 Prompt for 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. In `@tests/server/api_endpoints/test_profile_change_log_api_integration.py` around lines 73 - 87, The test currently validates the parsed ProfileChangeLogViewResponse model but doesn't explicitly verify that the `mentioned_profiles` field is absent from the raw JSON payload for the reconstructed row. Add an explicit assertion after obtaining the row from rows[run_id] that checks the raw JSON data from resp.json() to confirm the `mentioned_profiles` field is not present in that row's data, ensuring the API contract is properly locked and prevents unexpected fields from being served even if the model parsing might miss them.
🤖 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.
Nitpick comments:
In `@reflexio/lib/_profiles.py`:
- Around line 585-589: Remove the unused method signatures
get_distinct_generated_from_request_ids and
get_profiles_by_generated_from_request_id from the ChangeLogReadStorage
protocol. Since reconstruct_profile_change_log now uses
get_all_generated_profiles for reading additions, these per-request methods are
no longer needed and their removal will prevent new implementations from being
forced to implement legacy hooks that are no longer utilized.
In `@tests/server/api_endpoints/test_profile_change_log_api_integration.py`:
- Around line 73-87: The test currently validates the parsed
ProfileChangeLogViewResponse model but doesn't explicitly verify that the
`mentioned_profiles` field is absent from the raw JSON payload for the
reconstructed row. Add an explicit assertion after obtaining the row from
rows[run_id] that checks the raw JSON data from resp.json() to confirm the
`mentioned_profiles` field is not present in that row's data, ensuring the API
contract is properly locked and prevents unexpected fields from being served
even if the model parsing might miss them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: def9fb71-640f-460c-9997-8388a3ef7761
📒 Files selected for processing (26)
pyproject.tomlreflexio/lib/_lineage_parity.pyreflexio/lib/_lineage_parity_readers.pyreflexio/lib/_profiles.pyreflexio/models/api_schema/domain/entities.pyreflexio/models/api_schema/ui/converters.pyreflexio/models/api_schema/ui/entities.pyreflexio/server/services/storage/retention.pyreflexio/server/services/storage/sqlite_storage/_base.pyreflexio/server/services/storage/sqlite_storage/_extras.pyreflexio/server/services/storage/storage_base/_extras.pyscripts/lineage_b3_parity_check.pytests/e2e_tests/test_complete_workflows.pytests/e2e_tests/test_interaction_workflows.pytests/e2e_tests/test_profile_workflows.pytests/lib/test_lineage_parity.pytests/lib/test_lineage_parity_readers.pytests/lib/test_profiles_unit.pytests/models/test_view_models.pytests/server/api_endpoints/test_profile_change_log_api_integration.pytests/server/services/profile/test_dedup_always_soft_integration.pytests/server/services/profile/test_profile_generation_service.pytests/server/services/storage/test_lineage_b3_parity_classifier_integration.pytests/server/services/storage/test_lineage_b3_parity_gate_integration.pytests/server/services/storage/test_lineage_b3_reconstruct_changelog_integration.pytests/server/services/storage/test_storage_contract_extras.py
💤 Files with no reviewable changes (15)
- tests/server/services/storage/test_lineage_b3_parity_gate_integration.py
- tests/lib/test_profiles_unit.py
- reflexio/models/api_schema/ui/converters.py
- tests/lib/test_lineage_parity.py
- scripts/lineage_b3_parity_check.py
- reflexio/models/api_schema/ui/entities.py
- tests/server/services/storage/test_lineage_b3_parity_classifier_integration.py
- reflexio/server/services/storage/storage_base/_extras.py
- reflexio/lib/_lineage_parity.py
- reflexio/server/services/storage/retention.py
- reflexio/models/api_schema/domain/entities.py
- tests/models/test_view_models.py
- reflexio/server/services/storage/sqlite_storage/_extras.py
- tests/server/services/storage/test_storage_contract_extras.py
- reflexio/server/services/storage/sqlite_storage/_base.py
…(review-loop) Address /review-loop findings on the B3 Task 7 PR: - restore SQLite contract coverage for the KEPT delete_all_profile_change_logs / delete_profile_change_log_for_user methods. Deleting TestProfileChangeLogs wholesale dropped their only asserting test (the add/get tests had to go); the delete methods are retained for Task 8, so a kept abstract method must keep its guard. Seed the frozen table via direct SQL (add_profile_change_log is gone). - reword the get_profile_change_logs docstring so it doesn't conflate the Task 6 (stop write) / Task 7 (stop read + remove field) / Task 8 (drop table) boundaries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breaking change (B3 / Track A Task 7). Releases as 0.3.0 (pre-1.0 minor-as-breaking;
major_on_zero=false). Pairs with the enterprise PR that bumps this submodule pointer.BREAKING
mentioned_profilesis removed from theProfileChangeLog/ProfileChangeLogViewAPI + SDK response shape. Readadded_profiles/removed_profilesinstead. The field has always been an empty list (reconstruction never populated it) and was deprecated in T5.Changes
mentioned_profilesfrom the domain + view models, the reconstruction output, the UI converter, SDK exports + client, and the SQLite/Supabase/Postgres storage shapes. The frozen_DDLCREATE TABLE is untouched (Task 8).lib/_lineage_parity.py,scripts/lineage_b3_parity_check.py, the Task-4 gate + classifier tests; remove the storageget_profile_change_logs+add_profile_change_logmethods + converters; remove theRetentionTarget.lib/_profiles.py, renamedChangeLogReadStorage.@skip_low_prioritye2e tests off the removed storage read method.major_on_zero = falseso this pre-1.0 breaking change releases as 0.3.0.Kept for Task 8
The legacy
profile_change_logstable +_DDL, and thedelete_all/delete_for_userstorage methods (the GDPR remap + table rename/drop land in Task 8 — see the plan).Verification
3005 OS unit+integration tests pass; ruff + pyright clean; full collection clean (3666 OS + 2895 enterprise, no import errors from the deletions).
Summary by CodeRabbit
Release Notes
New Features
Breaking Changes
mentioned_profilesfrom profile change log API responses and view models.added_profiles,removed_profiles, andcreated_atmetadata.