feat(bootstrap): add SUPERSEDED_VALUES for config migration in brand-defaults - #1112
Merged
Merged
Conversation
…igration brand-defaults previously skipped keys with existing non-placeholder values, making it impossible to migrate configs through the pipeline. New SUPERSEDED_VALUES dict maps old defaults to new ones — when brand-defaults sees a superseded value, it auto-replaces it. First migration: QDRANT_COLLECTION pmoves_chunks → pmoves_chunks_qwen3 (384d MiniLM → 3072d Qwen3-4b embedding collection). Also adds QDRANT_COLLECTION to DEFAULTS for fresh installs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughA new default environment variable Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pmoves/tools/brand_defaults.py (1)
318-325: Clarify intent: migration silently succeeds even when new collection doesn't exist.The migration from
pmoves_chunks→pmoves_chunks_qwen3updates the config, but there's no validation thatpmoves_chunks_qwen3actually exists in Qdrant with data. Per the PR description, extract-worker restarts with the new collection, but:
- Existing embeddings in
pmoves_chunksare not migrated- No warning is emitted that users may need to re-index
Consider adding a log message when a superseded value is detected:
if new_val != current: + print(f"Migrating {key}: {current} → {new_val}", file=sys.stderr) text = _set_kv(text, key, new_val)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/tools/brand_defaults.py` around lines 318 - 325, The code silently replaces superseded config values (when current in SUPERSEDED_VALUES) without validating the target; update the branch in which new_val = SUPERSEDED_VALUES[current] (the block using current, SUPERSEDED_VALUES, _set_kv, key, text) to call into the Qdrant client to verify that the collection new_val exists and contains points before committing the change, and if the collection is missing or empty emit a warning log (e.g., via the module logger) stating that embeddings were not migrated and users may need to re-index; only call _set_kv and proceed if the collection check passes (or still set the value but ensure the warning is logged so the restart/re-index guidance is visible).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/tools/brand_defaults.py`:
- Around line 319-324: The codebase contains hardcoded fallbacks for the
QDRANT_COLLECTION env var that still point to "pmoves_chunks" even though
SUPERSEDED_VALUES migrates values to "pmoves_chunks_qwen3"; locate where
QDRANT_COLLECTION is read (e.g., default assignment in gateway modules and
seed/load scripts) and either change the literal fallback from "pmoves_chunks"
to "pmoves_chunks_qwen3" or remove the fallback so the code raises/config
validation fails when QDRANT_COLLECTION is unset; ensure the change is applied
consistently across usages that reference QDRANT_COLLECTION and aligns with the
SUPERSEDED_VALUES mapping.
- Around line 54-59: SUPERSEDED_VALUES contains an identity mapping
("all-MiniLM-L6-v2": "all-MiniLM-L6-v2") that will never be applied because the
migration logic (see the check new_val != current) skips identity mappings;
remove this key/value pair from SUPERSEDED_VALUES or replace it with a commented
note documenting the deprecation so it no longer appears as an executable
mapping.
---
Nitpick comments:
In `@pmoves/tools/brand_defaults.py`:
- Around line 318-325: The code silently replaces superseded config values (when
current in SUPERSEDED_VALUES) without validating the target; update the branch
in which new_val = SUPERSEDED_VALUES[current] (the block using current,
SUPERSEDED_VALUES, _set_kv, key, text) to call into the Qdrant client to verify
that the collection new_val exists and contains points before committing the
change, and if the collection is missing or empty emit a warning log (e.g., via
the module logger) stating that embeddings were not migrated and users may need
to re-index; only call _set_kv and proceed if the collection check passes (or
still set the value but ensure the warning is logged so the restart/re-index
guidance is visible).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a3f626fa-1e73-4870-b5ca-5865d1c04960
📒 Files selected for processing (1)
pmoves/tools/brand_defaults.py
Comment on lines
+54
to
+59
| SUPERSEDED_VALUES: dict[str, str] = { | ||
| # Qwen3-4b embedding migration: 384d → 3072d collection | ||
| "pmoves_chunks": "pmoves_chunks_qwen3", | ||
| # Legacy sentence-transformer model (replaced by TensorZero routing) | ||
| "all-MiniLM-L6-v2": "all-MiniLM-L6-v2", # keep as-is, TZ overrides at runtime | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other usages of "pmoves_chunks" as a value in env files
rg -n 'pmoves_chunks' --type-add 'env:*.{env,example,shared}' --type env
rg -n 'pmoves_chunks' -g '*.py' | grep -v 'SUPERSEDED_VALUES\|QDRANT_COLLECTION'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 466
🏁 Script executed:
# Look at the upsert_env function around line 322
rg -n 'def upsert_env' pmoves/tools/brand_defaults.py -A 50 | head -80Repository: POWERFULMOVES/PMOVES.AI
Length of output: 2483
🏁 Script executed:
# Also search for where SUPERSEDED_VALUES is referenced
rg -n 'SUPERSEDED_VALUES' pmoves/tools/brand_defaults.py -B 2 -A 2Repository: POWERFULMOVES/PMOVES.AI
Length of output: 705
🏁 Script executed:
# Check usage of "all-MiniLM-L6-v2" in codebase
rg -n 'all-MiniLM-L6-v2' --type pyRepository: POWERFULMOVES/PMOVES.AI
Length of output: 3994
Remove dead code: the identity mapping on line 58 never executes.
The "all-MiniLM-L6-v2": "all-MiniLM-L6-v2" entry is unreachable dead code. Line 322 explicitly checks if new_val != current: before applying migrations, so identity mappings are always skipped. Remove this entry or convert it to a comment if it's meant to document a deprecated migration.
Note: The value-based lookup for "pmoves_chunks" is not a practical concern—this string does not appear as a standalone value in other environment variables in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/tools/brand_defaults.py` around lines 54 - 59, SUPERSEDED_VALUES
contains an identity mapping ("all-MiniLM-L6-v2": "all-MiniLM-L6-v2") that will
never be applied because the migration logic (see the check new_val != current)
skips identity mappings; remove this key/value pair from SUPERSEDED_VALUES or
replace it with a commented note documenting the deprecation so it no longer
appears as an executable mapping.
Comment on lines
+319
to
324
| # Check if current value has been superseded by a migration | ||
| if current in SUPERSEDED_VALUES: | ||
| new_val = SUPERSEDED_VALUES[current] | ||
| if new_val != current: | ||
| text = _set_kv(text, key, new_val) | ||
| continue |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all hardcoded references to the old collection name
rg -n '"pmoves_chunks"' --type py -g '!brand_defaults.py'Repository: POWERFULMOVES/PMOVES.AI
Length of output: 522
Update hardcoded collection defaults in hi-rag-gateway services to match migration.
The superseded-value check correctly migrates existing configurations to "pmoves_chunks_qwen3". However, these services retain stale fallbacks that will silently use the old collection name if QDRANT_COLLECTION is unset:
pmoves/services/hi-rag-gateway/gateway.py:29pmoves/services/hi-rag-gateway-v2/app.py:26pmoves/services/hi-rag-gateway-v2/scripts/seed_local.py:10pmoves/services/hi-rag-gateway-v2/scripts/load_csv.py:16
Update all occurrences from "pmoves_chunks" to "pmoves_chunks_qwen3", or remove the fallback entirely to fail fast when the env var is missing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/tools/brand_defaults.py` around lines 319 - 324, The codebase contains
hardcoded fallbacks for the QDRANT_COLLECTION env var that still point to
"pmoves_chunks" even though SUPERSEDED_VALUES migrates values to
"pmoves_chunks_qwen3"; locate where QDRANT_COLLECTION is read (e.g., default
assignment in gateway modules and seed/load scripts) and either change the
literal fallback from "pmoves_chunks" to "pmoves_chunks_qwen3" or remove the
fallback so the code raises/config validation fails when QDRANT_COLLECTION is
unset; ensure the change is applied consistently across usages that reference
QDRANT_COLLECTION and aligns with the SUPERSEDED_VALUES mapping.
POWERFULMOVES
added a commit
that referenced
this pull request
Mar 27, 2026
…ity hardening Fleet Networking & RustDesk: - KVM2 self-hosted RustDesk relay (hbbs+hbbr, systemd, UFW-locked) - Scripts: fix-kvm2-rustdesk-relay.sh, restart-jetson-rustdesk.sh (env var driven — HOSTINGER_KVM2_IP, RUSTDESK_RELAY_KEY, JETSON_IPS) - RUSTDESK_SELF_HOSTED.md deployment guide (sanitized, no secrets) - QR code generation instructions for mobile enrollment Security Hardening: - Network hardening: localhost defaults for all service bindings - 4090 laptop host hardening script - SSH key-only auth across 8 nodes (password disabled) - Gitignore: rustdesk QR images excluded from repo Infrastructure: - DeepResearch Dockerfile: add pmoves.chit COPY for CGP_SPEC_VERSION (Python files only, secrets manifests excluded from image layers) - TOPOLOGY.md: KVM2 RustDesk ports, updated node hostnames, Jetson status - AGNOTE4482 ACK: Z890-CLAUDE fleet networking claim signed Also includes merged PR content: - Hi-RAG embedding defaults aligned to Qwen3 2560d (#1122) - Pinokio Codex plugin + Agent Zero launcher (#1121) - Publishing approval handoff state (#1120) - TTS MCP bridge + expression registry (#1116) - Damage-control hooks Windows compat (#1123) - AGNOTE4482 fleet claim docs (#1127) - Dependency bumps (#1111, #1113, #1128) - Bootstrap SUPERSEDED_VALUES (#1112) - Various docs updates (#1115, #1117, #1119, #1124, #1125) - Publisher RPC idempotent guards (#1126) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 28, 2026
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
2 participants
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.
Summary
SUPERSEDED_VALUESdict tobrand_defaults.pyfor automatic config migrationQDRANT_COLLECTIONfrompmoves_chunks(384d) topmoves_chunks_qwen3(3072d)QDRANT_COLLECTIONtoDEFAULTSfor fresh installsDesign
Previously,
upsert_env()skipped any key with a non-placeholder value. This made it impossible to migrate configs through the secrets pipeline — you had to manually edit env.shared.Now:
SUPERSEDED_VALUESmaps old defaults to new ones. The upsert loop checks if the current value is superseded before skipping.Test plan
make brand-defaultsmigratespmoves_chunks→pmoves_chunks_qwen3QDRANT_COLLECTION=pmoves_chunks_qwen3pmoves_chunks_qwen3as defaultGenerated with Claude Code
Summary by CodeRabbit