Conversation
Guide first-time setup through prerequisites, configuration, runtime validation, and a first processing run so blockers and recovery steps are clear.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e334038eef
ℹ️ 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".
| reachable = bool(runtime.get("reachable")) | ||
| state = "ready" if reachable and not missing_models else "blocked" |
There was a problem hiding this comment.
Handle runtime payloads that omit
reachable
build_runtime_view treats runtime.get("reachable") as the sole readiness signal, but the production GetRuntimeStatus path currently returns data from SummarizationEngine.runtime_status() without a reachable field (see service_summarize.py), so successful runtime checks are interpreted as unreachable. In that case setup_view.state is always "blocked", which keeps the setup flow from ever marking runtime complete and blocks the process step in the new gated UI even when models are present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Handled in 111a013. build_runtime_view() now treats a missing reachable field as a successful runtime status call while still honoring explicit False, which matches the current SummarizationEngine.runtime_status() payload shape. Added regression coverage in tests/test_setup_view_models.py for the production-shaped runtime payload without reachable.
Treat successful runtime status calls as reachable when older runtime adapters omit the field so the setup workflow can unblock runtime and processing correctly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 111a0137ed
ℹ️ 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".
video_rss_aggregator/static/setup.js
Outdated
| if (report.ready) { | ||
| setStatus(status.diagnostics, "All required dependencies are available."); | ||
| views.setStatus(status.diagnostics, "All required dependencies are available."); |
There was a problem hiding this comment.
Base diagnostics status on setup_view readiness
handleDiagnostics decides the banner message from report.ready, but that flag in api.py only reflects FFmpeg/FFprobe/yt-dlp/Ollama and does not include the new Python 3.11 gate added in build_diagnostics_view. When dependencies are present on Python 3.10, setup_view.state is "blocked" while this branch still shows "All required dependencies are available.", which gives a false success signal even though the workflow remains blocked.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Handled in 5c2a632. handleDiagnostics() now bases the success/error banner on diagnosticsView.state instead of raw report.ready, so the message stays aligned with the guided blocker logic (including the Python 3.11 gate). The same change set also relaxes the JS asset content-type assertions to accept both text/javascript and application/javascript, which fixes the Windows CI failure.
Use the shaped diagnostics view for setup status messaging and relax JS asset content-type assertions so the setup tests pass on both text/javascript and application/javascript responders.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c2a632144
ℹ️ 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".
| result.model || | ||
| result.model_name || | ||
| result.summary_model || | ||
| result.selected_model || |
There was a problem hiding this comment.
Read model_used from process payload when building summary
buildProcessSummaryView derives the displayed model via collectProcessModel, but that helper only checks top-level keys (model, model_name, etc.) and never reads summary.model_used, which is where /process currently reports the selected model (video_rss_aggregator/api.py). In normal successful runs this makes the new setup summary always show “Model not reported,” so users can’t verify which model actually processed the source.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Handled in e658e44. collectProcessModel() now reads the nested /process payload field at summary.model_used, so successful setup runs show the selected model in the guided summary. Added regression coverage in tests/test_setup_studio_contract.py for that mapping.
Read nested process payload model metadata from summary.model_used so successful setup runs show which model handled the source.
Summary
setup_viewpayloads plus regression coverage for setup routes, assets, and shell contractsTest Plan
python -m pytest -qruff check .python -m pytest tests/test_setup_view_models.py tests/test_api_setup.py tests/test_setup_studio_contract.py tests/adapters/test_api_app.py -q/health,/,/static/setup.css,/static/setup.js,/setup/config,/setup/diagnostics, and/rssNotes
http://127.0.0.1:11434