refactor(demo): provide execution settings solely via profile (RFC-0063) - #130
Conversation
![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: 8fab6c41d9
ℹ️ 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".
| conf_path = self.generated_solutions[path.name]["conf_path"] | ||
| command_str = "" | ||
| if profile and profile.get("dataset"): | ||
| command = self.demo_command(profile, conf_path.relative_to(PROJECT_ROOT), PROJECT_ROOT / "output" / path.stem) |
There was a problem hiding this comment.
When output/<solution> is unwritable or already contains a demo-profile.json symlink, this call throws only after save_solution has created the JSON/.conf pair and registered it in generated_solutions. The client therefore receives a failed save response even though the files persist, and retrying reports FILE_EXISTS; profile creation should occur inside the save transaction or failure should roll back the pair and session state.
Useful? React with 👍 / 👎.
| "config": "build/studio-test-g6f8oba0/configs/pipeline_fixture.conf", | ||
| "dataset": "/home/ubuntu/project/llm-ops-agy/data/corpus_entity_extract.txt", |
There was a problem hiding this comment.
This and the other newly added output/pipeline_*/demo-profile.json files are runtime test artifacts containing randomized temporary build paths and an author-specific absolute dataset path, so they are unusable in another checkout. Moreover, the corresponding Studio tests regenerate these stable output filenames with a new temporary-directory suffix on every run, leaving all of these tracked files modified; remove them and ensure the tests clean or ignore their output directories.
Useful? React with 👍 / 👎.
| } else if (lower == "cpu") { | ||
| *out_type = ComputePlatform::kCpu; |
There was a problem hiding this comment.
After this branch accepts only cpu, the active doc/kitellm.md guidance at lines 54–55 still tells alg_demo users to use cpu_generic. A user following that documented setup in a Profile now fails LoadAndValidateProfilesDocument as unsupported instead of running the Kite demo, so migrate that remaining guide to the canonical cpu name as part of the alias removal.
Useful? React with 👍 / 👎.
1 participant
Verified by the repository's canonical local quality gate.