Skip to content

Commit dfa069a

Browse files
committed
fix: address codex review for PR2 (wizard)
- Improve field removal to clean up renames by both old_name and new_name - Resolve update names through rename map in working schema preview - Add multi-prefix guard to reject indexes with multiple prefixes - Fix dependent prompts (UNF, no_index) when field is already sortable - Pass existing field attrs to common attrs prompts for update mode
1 parent deb1158 commit dfa069a

File tree

1 file changed

+56
-11
lines changed

1 file changed

+56
-11
lines changed

redisvl/migration/wizard.py

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
class MigrationWizard:
2222
def __init__(self, planner: Optional[MigrationPlanner] = None):
2323
self.planner = planner or MigrationPlanner()
24+
self._existing_sortable: bool = False
2425

2526
def run(
2627
self,
@@ -45,6 +46,15 @@ def run(
4546
)
4647
source_schema = IndexSchema.from_dict(snapshot.schema_snapshot)
4748

49+
# Guard: the wizard does not support indexes with multiple prefixes.
50+
prefixes = source_schema.index.prefix
51+
if isinstance(prefixes, list) and len(prefixes) > 1:
52+
raise ValueError(
53+
f"Index '{resolved_index_name}' has multiple prefixes "
54+
f"({prefixes}). The migration wizard only supports single-prefix "
55+
"indexes. Use the planner API directly for multi-prefix indexes."
56+
)
57+
4858
print(f"Building a migration plan for index '{resolved_index_name}'")
4959
self._print_source_schema(source_schema.to_dict())
5060

@@ -156,8 +166,13 @@ def _apply_staged_changes(
156166
if field["name"] in rename_map:
157167
field["name"] = rename_map[field["name"]]
158168

159-
# Apply updates (reflect attribute changes in working schema)
160-
update_map = {u.name: u for u in changes.update_fields}
169+
# Apply updates (reflect attribute changes in working schema).
170+
# Resolve update names through the rename map so that updates staged
171+
# before a rename (referencing the old name) still match.
172+
update_map = {}
173+
for u in changes.update_fields:
174+
resolved = rename_map.get(u.name, u.name)
175+
update_map[resolved] = u
161176
for field in working["fields"]:
162177
if field["name"] in update_map:
163178
upd = update_map[field["name"]]
@@ -245,12 +260,27 @@ def _build_patch(
245260
print(f"Cancelled staged addition of '{field_name}'.")
246261
else:
247262
changes.remove_fields.append(field_name)
248-
# Also remove any queued updates or renames for this field
263+
# Also remove any queued updates or renames for this field.
264+
# Check both old_name and new_name so that:
265+
# - renames FROM this field are dropped (old_name match)
266+
# - renames TO this field are dropped (new_name match)
267+
# Also drop updates referencing either the field itself or
268+
# any pre-rename name that mapped to it.
269+
rename_aliases = {field_name}
270+
for r in changes.rename_fields:
271+
if r.new_name == field_name:
272+
rename_aliases.add(r.old_name)
273+
if r.old_name == field_name:
274+
rename_aliases.add(r.new_name)
249275
changes.update_fields = [
250-
u for u in changes.update_fields if u.name != field_name
276+
u
277+
for u in changes.update_fields
278+
if u.name not in rename_aliases
251279
]
252280
changes.rename_fields = [
253-
r for r in changes.rename_fields if r.old_name != field_name
281+
r
282+
for r in changes.rename_fields
283+
if r.old_name != field_name and r.new_name != field_name
254284
]
255285
elif action == "4":
256286
# Filter out staged additions from rename candidates
@@ -357,7 +387,11 @@ def _prompt_update_field(
357387
if selected["type"] == "vector":
358388
attrs = self._prompt_vector_attrs(selected)
359389
else:
360-
attrs = self._prompt_common_attrs(selected["type"], allow_blank=True)
390+
attrs = self._prompt_common_attrs(
391+
selected["type"],
392+
allow_blank=True,
393+
existing_attrs=selected.get("attrs"),
394+
)
361395
if not attrs:
362396
print("No changes collected.")
363397
return None
@@ -485,7 +519,10 @@ def _prompt_change_prefix(self, source_schema: Dict[str, Any]) -> Optional[str]:
485519
return new_prefix
486520

487521
def _prompt_common_attrs(
488-
self, field_type: str, allow_blank: bool = False
522+
self,
523+
field_type: str,
524+
allow_blank: bool = False,
525+
existing_attrs: Optional[Dict[str, Any]] = None,
489526
) -> Dict[str, Any]:
490527
attrs: Dict[str, Any] = {}
491528

@@ -511,6 +548,11 @@ def _prompt_common_attrs(
511548
if index_empty is not None:
512549
attrs["index_empty"] = index_empty
513550

551+
# Track whether the field was already sortable so that type-specific
552+
# prompt helpers (text UNF, numeric UNF) can offer dependent prompts
553+
# even when the user leaves sortable blank during an update.
554+
self._existing_sortable = (existing_attrs or {}).get("sortable", False)
555+
514556
# Type-specific attributes
515557
if field_type == "text":
516558
self._prompt_text_attrs(attrs, allow_blank)
@@ -519,8 +561,11 @@ def _prompt_common_attrs(
519561
elif field_type == "numeric":
520562
self._prompt_numeric_attrs(attrs, allow_blank, sortable)
521563

522-
# No index - only meaningful with sortable
523-
if sortable or (allow_blank and attrs.get("sortable")):
564+
# No index - only meaningful with sortable.
565+
# When updating (allow_blank), also check the existing field's sortable
566+
# state so we offer dependent prompts even if the user left sortable blank.
567+
_existing_sortable = self._existing_sortable
568+
if sortable or (allow_blank and (_existing_sortable or attrs.get("sortable"))):
524569
print(" No index: store field for sorting only, not searchable")
525570
no_index = self._prompt_bool("No index", allow_blank=allow_blank)
526571
if no_index is not None:
@@ -560,7 +605,7 @@ def _prompt_text_attrs(self, attrs: Dict[str, Any], allow_blank: bool) -> None:
560605
attrs["phonetic_matcher"] = phonetic
561606

562607
# UNF (only if sortable)
563-
if attrs.get("sortable"):
608+
if attrs.get("sortable") or self._existing_sortable:
564609
print(" UNF: preserve original form (no lowercasing) for sorting")
565610
unf = self._prompt_bool("UNF (un-normalized form)", allow_blank=allow_blank)
566611
if unf is not None:
@@ -585,7 +630,7 @@ def _prompt_numeric_attrs(
585630
) -> None:
586631
"""Prompt for numeric field specific attributes."""
587632
# UNF (only if sortable)
588-
if sortable or attrs.get("sortable"):
633+
if sortable or attrs.get("sortable") or self._existing_sortable:
589634
print(" UNF: preserve exact numeric representation for sorting")
590635
unf = self._prompt_bool("UNF (un-normalized form)", allow_blank=allow_blank)
591636
if unf is not None:

0 commit comments

Comments
 (0)