Skip to content

fix: Add missing Segment import in test

3eb06b6
Select commit
Loading
Failed to load commit list.
Open

fix: Enhance EFV audit logs #7535

fix: Add missing Segment import in test
3eb06b6
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 22, 2026 in 9m 46s

Code review found 2 important issues

Found 5 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 2
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important api/features/versioning/tasks.py:281-302 Removed feature states omitted from audit log
🔴 Important api/features/versioning/tasks.py:264-272 Audit log summary omits value and multivariate changes

Annotations

Check failure on line 302 in api/features/versioning/tasks.py

See this annotation in the file changed.

@claude claude / Claude Code Review

Removed feature states omitted from audit log

The new audit log silently omits removed feature states. `get_updated_feature_states_for_version()` only iterates the **new** version's feature states and compares them against the previous version — it never walks the previous version looking for keys that don't appear in the new one. So when a user publishes a version whose only change is dropping a segment override (e.g. via `segment_ids_to_delete_overrides`), the removed override is never visited and the audit log falls back to just the head

Check failure on line 272 in api/features/versioning/tasks.py

See this annotation in the file changed.

@claude claude / Claude Code Review

Audit log summary omits value and multivariate changes

The audit-log summary only reports each changed feature state's scope and current `enabled`/`disabled` flag, but `get_updated_feature_states_for_version` also flags states whose `feature_state_value` or multivariate values changed while `enabled` stayed the same. Such value-only edits produce a line indistinguishable from an enabled-toggle change (e.g. `- Environment default: enabled`), defeating this PR's stated goal of making EFV audit logs less opaque. Consider including the value/MV diff (or