feat(cli): salvage generic scaffold refresh for review#27
feat(cli): salvage generic scaffold refresh for review#27blackopsrepl merged 2 commits intorelease/0.6.0from
Conversation
blackopsrepl
force-pushed
the
codex/release-0.6.0-scaffold-salvage
branch
from
4ffc147 to
1279239
Compare
![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: 12792399cf
ℹ️ 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".
| pub fn new(item_facts: Vec<Item>, containers: Vec<Container>) -> Self { | ||
| let all_item_indices = (0..item_facts.len()).collect(); | ||
| Self { item_facts, containers, all_item_indices, score: None } |
There was a problem hiding this comment.
all_item_indices aligned with posted item IDs
Because the list scaffold exposes ItemDto.index as the item identifier, callers will naturally send their own IDs (for example database keys). Plan::new() discards those IDs and always rebuilds all_item_indices as 0..len, so any container entry that still contains the original IDs is now absent from the list variable’s element collection. In that case /schedules and /schedules/analyze will solve or analyze a different problem than the one the client submitted, typically dropping every non-zero-based item from the sequence model.
Useful? React with 👍 / 👎.
| let name_to_idx: std::collections::HashMap<&str, usize> = | ||
| item_facts.iter().map(|i| (i.name.as_str(), i.index)).collect(); |
There was a problem hiding this comment.
This rebuilds each container’s items list through a HashMap<&str, usize> keyed on Item.name. That makes the new list scaffold non-round-trippable as soon as two items share the same display name: both positions resolve to the last matching item index, so one item disappears and another is duplicated when /schedules or /schedules/analyze parses the payload. The generic template does not require unique names, so this will bite real users who load repeated labels like "Task" or "Order".
Useful? React with 👍 / 👎.
1 participant
Summary
--basicpath--listscaffold while keeping--list=vehicle-routingsolverforge-ui = "0.2.0"in the salvaged generic templatesTesting
Review Notes
release/0.6.0after PR fix(release): port logging, score parsing, and sf-config wiring #26