feat: support OSS-backed model deployment#1352
Conversation
WalkthroughAdds a FastAPI deployment service and web UI, S3-compatible model discovery, OSS-aware vLLM Kubernetes generation, safer vLLM argument rendering, documentation, tests, and a multi-stage Docker image. ChangesService API and deployment UI
S3 model source
vLLM Kubernetes generation
Service container
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (3)
tests/unit/sdk/test_s3_models.py (1)
35-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the default-source switch.
No test pins
is_s3_model_source_configured()'s behavior whenAIC_MODEL_SOURCEis unset, norget_configured_s3_model_uris(). Given this default materially changesget_default_models()behavior app-wide, a test here would make the intended default explicit and guard against accidental flips.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/sdk/test_s3_models.py` around lines 35 - 68, Add unit coverage for is_s3_model_source_configured() when AIC_MODEL_SOURCE is unset, asserting the intended default source behavior, and cover get_configured_s3_model_uris() for the corresponding configuration. Use environment monkeypatching and preserve the existing cache-clearing fixture so the tests verify the default that get_default_models() will use.src/aiconfigurator/generator/rendering/engine.py (1)
538-544: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
k8s_contextcan be silently reset, dropping the vLLM filter if engine templates are ever added for vLLM.The
has_engine_templatesbranch reassignsk8s_context = dict(context)(568-570 region) without reapplying_filter_vllm_k8s_cli_args. Currently harmless since vLLM ships noextra_engine_args*.yaml.j2templates, but if that changes later this silently reintroduces unsafe flags into the vLLM K8s manifest. Consider re-applying the filter after anyk8s_contextreset, or guarding the reset to skip vLLM.Also applies to: 562-578
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiconfigurator/generator/rendering/engine.py` around lines 538 - 544, Ensure the has_engine_templates branch in the rendering flow does not discard the vLLM CLI-argument filtering applied to k8s_context. When it reassigns k8s_context from context, reapply _filter_vllm_k8s_cli_args for each worker when backend is vllm and deployment_target is dynamo-j2 or dynamo-python, or avoid that reset for this case while preserving existing template handling.src/aiconfigurator/service/app.py (1)
787-806: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftSync kubectl calls with up to 1800s timeout can starve FastAPI's threadpool under concurrent deploys.
ApplyDeploymentRequest.timeout_secondsallows up to 1800s, and these are syncdefroutes, so each longapply/delete/health-check call occupies a threadpool worker for its full duration. A handful of concurrent long deployments could exhaust the pool and stall unrelated requests.Consider capping the practical timeout lower, or moving to
asyncio.create_subprocess_exec/ a background job queue so long-running kubectl operations don't block request-handling threads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiconfigurator/service/app.py` around lines 787 - 806, Update the synchronous kubectl execution flow centered on _run_kubectl so deployment, deletion, and health-check operations cannot occupy FastAPI’s request threadpool for the full 1800-second request limit. Prefer moving kubectl execution to an asynchronous subprocess or background-job mechanism; otherwise enforce a substantially lower practical timeout consistently with ApplyDeploymentRequest.timeout_seconds and the affected routes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/Dockerfile.service`:
- Around line 26-28: Remove internal infrastructure defaults from
docker/Dockerfile.service lines 26-28 by replacing AIC_MODEL_S3_BUCKET,
AWS_ENDPOINT_URL, and AWS_DEFAULT_REGION with generic placeholders or requiring
explicit build/runtime configuration. Also update
src/aiconfigurator/service/static/index.html lines 605-643 to remove the
hardcoded internal image registry, node hostnames, OSS endpoint, region, and
secret name, using generic placeholders or blank values that require explicit
configuration.
In `@src/aiconfigurator/generator/aggregators.py`:
- Around line 73-75: Update the explicit-name handling near `explicit_name` and
`name` to enforce RFC 1123 DGD naming: lowercase alphanumeric characters and
hyphens only, with a maximum length of 63 characters. Choose and consistently
implement either rejecting invalid explicit names or normalizing them, and add
coverage for invalid characters and overlong names while preserving generated
fallback names.
In
`@src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2`:
- Around line 315-318: Remove the explicit --router-mode round-robin argument
from the OSS frontend command in the vllm deployment template, allowing
DYN_ROUTER_MODE=kv to control routing when enable_router is enabled. Preserve
the existing model-name and model-path arguments.
- Around line 158-164: Update the metadata download loop in the vLLM deployment
template so tokenizer.json and tokenizer_config.json are optional, matching the
existing optional handling for generation_config.json; only config.json must
remain required and download failures for it should still propagate. Apply the
same behavior to both affected loop blocks.
In `@src/aiconfigurator/generator/config/deployment_config.yaml`:
- Around line 23-29: Remove the circular defaults between
ServiceConfig.model_name and ServiceConfig.served_model_name in the deployment
configuration. Make one field authoritative and have the other derive from it in
only one direction, ensuring schema evaluation cannot leave both values unset.
- Around line 80-83: Update the vllm image selection around
ServiceConfig.model_path and generator_dynamo_version so OSS deployments do not
always use the hardcoded Dynamo 1.1.1 image. Resolve the runtime image using the
backend-version compatibility matrix for the requested generator_dynamo_version,
and reject unsupported backend/version combinations instead of emitting an
incompatible image.
- Around line 136-141: Update the deployment entries for
K8sConfig.frontend_node_selector and K8sConfig.worker_node_selector to declare
vLLM as their supported backend, while preserving their optional status and
empty-object defaults.
In `@src/aiconfigurator/sdk/s3_models.py`:
- Around line 72-101: The list_s3_models function performs an uncached
full-bucket scan on every invocation. Add a short-lived or per-process
memoization strategy around list_s3_models, keyed by its bucket input, while
preserving the existing pagination, sorting, and S3 error behavior; provide the
project’s established cache invalidation mechanism if one exists.
- Around line 123-124: Require explicit S3 opt-in in
is_s3_model_source_configured() by removing the implicit "s3" environment
default. In get_s3_client() and list_s3_models(), stop falling back to the
hardcoded endpoint, region, and bucket; instead require configuration and raise
clear errors when values are absent. Update docs/dynamo_deployment_guide.md to
use generic placeholders rather than those backend defaults.
- Around line 46-60: Update get_s3_client to configure explicit, bounded
botocore connect and read timeouts plus a capped retry policy in its Config,
while preserving the existing endpoint, region, and path-style addressing
behavior.
In `@src/aiconfigurator/service/app.py`:
- Around line 453-461: Update _attach_and_store_jid to persist each newly
generated run through _save_run_record instead of only calling
_cache_transient_run. Ensure generation-only runs are available after process
restarts or on other replicas, and remove or stop relying on the unbounded
_TRANSIENT_RUNS cache for this flow while preserving the existing JID attachment
behavior.
- Around line 69-79: Sanitize the attacker-controlled request path before
passing it to logger.exception in the add_trace_id middleware. Escape or strip
newline and other control characters from request.url.path, then use the
sanitized value in the extra metadata while preserving the existing trace ID and
error handling.
- Around line 824-837: Update the deployment/statefulset/daemonset branch in
_check_k8s_health to catch APIError from the rollout-status _run_kubectl call
and return an unhealthy health entry containing the rollout failure details
instead of propagating the exception. Preserve the successful response for
completed rollouts so _apply_k8s_deployment can still build its payload and mark
the deployment record as deployed, and add coverage for the failed or timed-out
rollout path.
- Around line 486-503: Redact sensitive credentials before storing run records:
update both _cache_transient_run and _save_run_record to pass request data
through the existing secret-redaction wrapper before normalization and
persistence. Ensure hf_token and any other supported secrets are removed or
masked consistently, so get_run cannot return plaintext credentials while
preserving the rest of the request record.
- Around line 713-763: Add an in-app authorization check at the entry points for
_apply_k8s_deployment and _delete_k8s_deployment before invoking kubectl, using
the service’s existing authz mechanism. Validate the resources returned by
_parse_k8s_resources, including kind, name, and namespace, and reject flag-like
or otherwise invalid identifiers before constructing any kubectl argv. Ensure
the same validated identifiers are used by _check_k8s_health and related
rollout/get commands.
In `@src/aiconfigurator/service/static/index.html`:
- Around line 827-842: Update deriveServedModelName and deriveDeploymentName to
generate RFC1123-compliant names: lowercase letters, digits, and hyphens only,
with each resulting name capped at 63 characters. Replace the current sanitizer
behavior that preserves dots and underscores, and ensure truncation does not
leave invalid leading or trailing hyphens.
In `@tests/unit/generator/test_vllm_oss_deployment.py`:
- Around line 33-73: Add a generator-level test alongside
test_vllm_oss_deployment_uses_model_express_and_secret_refs that uses an s3://
model path and runs apply_defaults followed by generate_backend_artifacts.
Validate the emitted k8s_deploy.yaml or golden artifact contains the expected
OSS endpoint, secret reference, model URI, and ModelExpress settings instead of
only rendering k8s_deploy.yaml.j2 directly.
---
Nitpick comments:
In `@src/aiconfigurator/generator/rendering/engine.py`:
- Around line 538-544: Ensure the has_engine_templates branch in the rendering
flow does not discard the vLLM CLI-argument filtering applied to k8s_context.
When it reassigns k8s_context from context, reapply _filter_vllm_k8s_cli_args
for each worker when backend is vllm and deployment_target is dynamo-j2 or
dynamo-python, or avoid that reset for this case while preserving existing
template handling.
In `@src/aiconfigurator/service/app.py`:
- Around line 787-806: Update the synchronous kubectl execution flow centered on
_run_kubectl so deployment, deletion, and health-check operations cannot occupy
FastAPI’s request threadpool for the full 1800-second request limit. Prefer
moving kubectl execution to an asynchronous subprocess or background-job
mechanism; otherwise enforce a substantially lower practical timeout
consistently with ApplyDeploymentRequest.timeout_seconds and the affected
routes.
In `@tests/unit/sdk/test_s3_models.py`:
- Around line 35-68: Add unit coverage for is_s3_model_source_configured() when
AIC_MODEL_SOURCE is unset, asserting the intended default source behavior, and
cover get_configured_s3_model_uris() for the corresponding configuration. Use
environment monkeypatching and preserve the existing cache-clearing fixture so
the tests verify the default that get_default_models() will use.
🪄 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: Enterprise
Run ID: 9a224db8-66bc-494a-9273-55ed7d722df2
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (20)
docker/Dockerfile.servicedocs/dynamo_deployment_guide.mdpyproject.tomlsrc/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2src/aiconfigurator/generator/config/deployment_config.yamlsrc/aiconfigurator/generator/rendering/engine.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/sdk/s3_models.pysrc/aiconfigurator/sdk/utils.pysrc/aiconfigurator/service/__init__.pysrc/aiconfigurator/service/app.pysrc/aiconfigurator/service/main.pysrc/aiconfigurator/service/models.pysrc/aiconfigurator/service/static/index.htmltests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.pytests/unit/generator/test_vllm_oss_deployment.pytests/unit/sdk/test_s3_models.pytests/unit/service/test_api.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (24)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use the project’s
uv-managed environment and run linting withruff check .andruff format --check ..
Files:
src/aiconfigurator/service/__init__.pysrc/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/service/main.pytests/unit/generator/test_vllm_oss_deployment.pysrc/aiconfigurator/sdk/utils.pytests/unit/sdk/test_s3_models.pytests/unit/generator/test_aggregators.pysrc/aiconfigurator/sdk/s3_models.pysrc/aiconfigurator/sdk/common.pytests/unit/generator/test_vllm_minimal_k8s_args.pysrc/aiconfigurator/service/models.pysrc/aiconfigurator/generator/rendering/engine.pytests/unit/service/test_api.pysrc/aiconfigurator/service/app.py
**/*
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
src/aiconfigurator/service/__init__.pysrc/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/service/main.pytests/unit/generator/test_vllm_oss_deployment.pydocs/dynamo_deployment_guide.mdsrc/aiconfigurator/sdk/utils.pytests/unit/sdk/test_s3_models.pytests/unit/generator/test_aggregators.pydocker/Dockerfile.servicesrc/aiconfigurator/service/static/index.htmlsrc/aiconfigurator/sdk/s3_models.pysrc/aiconfigurator/sdk/common.pysrc/aiconfigurator/generator/config/deployment_config.yamltests/unit/generator/test_vllm_minimal_k8s_args.pysrc/aiconfigurator/service/models.pypyproject.tomlsrc/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2src/aiconfigurator/generator/rendering/engine.pytests/unit/service/test_api.pysrc/aiconfigurator/service/app.py
src/aiconfigurator/generator/**
📄 CodeRabbit inference engine (.claude/rules/generator/config_schema.md)
src/aiconfigurator/generator/**: Update artifact templates when a parameter requires special handling in generated outputs.
Validate configuration changes by running the generator with the parameter set and omitted, running the generator validator against backend API schemas, and checking--generator-helpoutput.When modifying generator code, review the cross-module impact map for affected modules outside
src/aiconfigurator/generator/.
src/aiconfigurator/generator/**: For generator bugs, trace backwards from the generated output through template rendering, rule evaluation, parameter mapping, default application, input parsing, and schema loading.
Apply fixes at the stage where the defect originates rather than patching templates for rule or data-mapping bugs.
When fixing a generator issue, check all backends and all version templates for the same problem pattern.
After a generator fix, reproduce the original failure, run the generator validator, check other backends and versions, and add a regression test.
src/aiconfigurator/generator/**: TRT-LLM: Placebuild_configaccording to the selected version: nested before 1.2.0rc5 and top-level from 1.2.0rc5 onward.
TRT-LLM: Keep template variables as top-level keys becauseengine.pyflattens the context.
TRT-LLM: Emitcache_transceiver_configaccording to the applicable version and interface.
TRT-LLM: Setcache_transceiver_config.backendtoDEFAULT.
TRT-LLM: Put engine parameters in--override-engine-argsJSON rather than unsupported direct CLI flags.
Ensuremax_num_tokens % tokens_per_block == 0.
Ensurecache_transceiver_max_tokens_in_bufferis aligned to the block size.
Setdisable_overlap_scheduler=truefor prefill andfalsefor decode/aggregate modes.
For MoE models, setTP = moe_tp * moe_ep.
Map KV cache dtypefloat16orbfloat16toautofor TRT-LLM.
For SGLang, do not emit--moe-dense-tp-sizeunless its value is 1 or absent.
Use KV transfer backendnixlby default for SGL...
Files:
src/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/generator/config/deployment_config.yamlsrc/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2src/aiconfigurator/generator/rendering/engine.py
⚙️ CodeRabbit configuration file
src/aiconfigurator/generator/**: - Enforce the generator development rules from.claude/rules/generator-development.md.
- Trace changes across the pipeline: input parsing, defaults, rule evaluation, parameter mapping, template rendering, and emitted artifacts.
- For
.j2templates and.rulefiles, verify# Guard:and# Why:comments are preserved and followed. Flag removals or bypasses unless the PR explicitly justifies them.- Check cross-backend consistency across TRT-LLM, vLLM, and SGLang when a backend-specific rule, template, parameter, or versioned artifact changes.
- For new user-facing generator parameters, require matching CLI, SDK/profiler bridge, generator validator, docs, and test coverage updates where applicable.
- For versioned template changes, verify the affected backend versions against
src/aiconfigurator/generator/config/backend_version_matrix.yaml.
Files:
src/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/generator/config/deployment_config.yamlsrc/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2src/aiconfigurator/generator/rendering/engine.py
src/aiconfigurator/generator/aggregators.py
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
Update aggregators when renamed parameters are aggregated, and add backward-compatible aliases here when an old parameter name was user-facing.
Files:
src/aiconfigurator/generator/aggregators.py
src/aiconfigurator/generator/**/*.py
📄 CodeRabbit inference engine (.claude/rules/generator/new_backend_version.md)
Do not hardcode backend version checks in the rendering engine; use version-template naming and fallback logic instead of conditions such as
if version >= ...in Python.
src/aiconfigurator/generator/**/*.py: For pure logic changes involving expressions or calculations, add unit-test coverage.
For new parameters or flags, add validator-test coverage.
Files:
src/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/generator/rendering/engine.py
src/aiconfigurator/generator/**/*.{py,jinja,j2,yaml,yml,json}
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
For template, rule, or schema changes that affect generated output, add integration tests and update golden snapshots.
Files:
src/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/generator/config/deployment_config.yamlsrc/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2src/aiconfigurator/generator/rendering/engine.py
src/aiconfigurator/generator/**/*
📄 CodeRabbit inference engine (.claude/rules/repo-guide.md)
src/aiconfigurator/generator/**/*: When editing or reviewingsrc/aiconfigurator/generator/**, load and apply the generator rules, including.claude/rules/generator-development.md.
Do not drift into deployment-config topics unless the task actually targetssrc/aiconfigurator/generator/.
Files:
src/aiconfigurator/generator/aggregators.pysrc/aiconfigurator/generator/config/deployment_config.yamlsrc/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2src/aiconfigurator/generator/rendering/engine.py
tests/unit/generator/**
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
Update generator test fixtures and assertions when parameters are renamed.
Files:
tests/unit/generator/test_vllm_oss_deployment.pytests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.py
tests/unit/generator/**/*.py
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
tests/unit/generator/**/*.py: Use fast unit tests for pure generator logic, testing individual functions in isolation and mocking schema loading and template rendering.
Test generator edge cases including MoE, disaggregated mode, aggregated mode, speculative decoding, prefix caching, minimal configuration, PVC configuration, each backend, and benchmark mode.
Files:
tests/unit/generator/test_vllm_oss_deployment.pytests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.py
tests/**/*.{py,yaml,txt,sh}
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
Use integration tests for the full input-to-artifacts pipeline, comparing output against golden snapshots without external dependencies.
Files:
tests/unit/generator/test_vllm_oss_deployment.pytests/unit/sdk/test_s3_models.pytests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.pytests/unit/service/test_api.py
tests/unit/generator/test_*.py
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
Place generator unit tests in tests/unit/generator/ and use descriptive test files such as test_aggregators.py, test_rule_engine.py, and backend-specific CLI argument tests.
Files:
tests/unit/generator/test_vllm_oss_deployment.pytests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run unit tests with
pytest -m unit; run the PR build subset withpytest -m "unit or build"when applicable.
Files:
tests/unit/generator/test_vllm_oss_deployment.pytests/unit/sdk/test_s3_models.pytests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.pytests/unit/service/test_api.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: - Check that tests cover the changed behavior rather than only the happy path.
- Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.
Files:
tests/unit/generator/test_vllm_oss_deployment.pytests/unit/sdk/test_s3_models.pytests/unit/generator/test_aggregators.pytests/unit/generator/test_vllm_minimal_k8s_args.pytests/unit/service/test_api.py
docs/**
⚙️ CodeRabbit configuration file
docs/**: - Check that docs match changed CLI, SDK, generator, backend, and support-matrix behavior.
- Flag docs that describe unsupported runtimes, stale command names, or behavior not covered by tests or support-matrix evidence.
Files:
docs/dynamo_deployment_guide.md
src/aiconfigurator/sdk/**
⚙️ CodeRabbit configuration file
src/aiconfigurator/sdk/**: - Verify SDK API changes remain compatible with generator inputs, profiler data flow, and documented examples.
- Flag silent schema or field-name drift between SDK models and generator/module bridge code.
Files:
src/aiconfigurator/sdk/utils.pysrc/aiconfigurator/sdk/s3_models.pysrc/aiconfigurator/sdk/common.py
tests/unit/generator/test_aggregators.py
📄 CodeRabbit inference engine (.claude/rules/generator/testing.md)
Cover parameter collection and default behavior in aggregator tests.
Files:
tests/unit/generator/test_aggregators.py
src/aiconfigurator/generator/config/deployment_config.yaml
📄 CodeRabbit inference engine (.claude/rules/generator/config_schema.md)
src/aiconfigurator/generator/config/deployment_config.yaml: Define deployment configuration entries with a section-qualifiedkey,requiredstatus, default, optionalbackend_defaults, and optional backend restrictions.
Place new parameters in the correct section, determine whether they are required or optional, define static or computed defaults, and specify backend support.
For computed defaults, verify referenced variables are available during default evaluation and handle missing orNonevalues safely.
Quote string Jinja2 defaults with double quotes inside single quotes, such asdefault: '"my-string"'.
Guard default expressions against missing values, for example usemodel_path.split("/")[-1] if model_path else "".
Avoid circular default references between parameters because the schema loader may produceNonefor both; verify the dependency chain is acyclic.Define new parameters, defaults, and backend support in
deployment_config.yaml. Update parameter keys when renaming parameters.Verify parameter default expressions and applicable
backend_defaultswhen a value is missing orNone.For new user-facing parameters, add a schema entry with section, default, required status, and backend-specific defaults where behavior differs; verify default container-image expressions and version-tag resolution.
Add new input parameters and their defaults to the deployment configuration schema.
Files:
src/aiconfigurator/generator/config/deployment_config.yaml
src/aiconfigurator/generator/config/**/*.yaml
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
When modifying template output, verify generated Kubernetes manifests and engine configuration files against their required schemas.
Files:
src/aiconfigurator/generator/config/deployment_config.yaml
src/aiconfigurator/generator/config/*.{j2,yaml}
📄 CodeRabbit inference engine (.claude/rules/generator/new_backend_version.md)
Create version-specific templates only when the backend CLI interface, engine configuration format, or startup command changes; do not create templates for no-op version bumps.
Files:
src/aiconfigurator/generator/config/deployment_config.yaml
src/aiconfigurator/generator/**/*.{j2,rule}
📄 CodeRabbit inference engine (.claude/rules/generator-development.md)
Before modifying any
.j2template or.rulefile, read ALL comments in the file. Do not silently remove or bypass guard comments marked with# Guard:or# Why:. If a requested change contradicts a guarded comment, quote the comment, explain why it exists, and ask for confirmation before proceeding.
Files:
src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2
src/aiconfigurator/generator/config/backend_templates/**
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
Add or update version-specific backend templates when a new backend version changes CLI behavior.
Files:
src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2
src/aiconfigurator/generator/**/*.j2
📄 CodeRabbit inference engine (.claude/rules/generator/cross_module_impact.md)
When changing generated run scripts, verify Bash syntax with
bash -n generated_run.sh; when changing Kubernetes output, verify YAML withkubectl apply --dry-run.
src/aiconfigurator/generator/**/*.j2: For a changed backend interface, copy the closest prior template, update flags or conditionals, and preserve the prior version template unchanged.
For deprecated backend CLI flags, retain the old flag in templates before the compatibility boundary and use the replacement flag in templates at or after the boundary.
Do not modify old version templates when introducing a replacement for a deprecated or renamed flag; create a new version-specific template instead.
Rely on template selection order—exact version, closest prior version, then base template—instead of creating a template for every version.
Files:
src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2
src/aiconfigurator/generator/config/backend_templates/**/*.j2
📄 CodeRabbit inference engine (.claude/rules/generator/debugging.md)
src/aiconfigurator/generator/config/backend_templates/**/*.j2: When investigating incorrect generated flags, blocks, or YAML structure, inspect the responsible Jinja template and verify its variables match the rendering context.
When changing templates, inspect every version-specific template for stale variable references, deprecated flags, and mode guards.
src/aiconfigurator/generator/config/backend_templates/**/*.j2: Keep backend-specific Jinja2 templates standalone; do not extract shared templates or macros.
When adding a new backend version template, copy the closest prior version and modify the copy; never edit prior-version templates.
Apply fixes to all affected version-specific templates and enumerate them explicitly.
Do not add a template variable unless its source can be traced through the pipeline.
Verify every variable referenced by a modified template comes from deployment input, a rule plugin, backend mapping, or the rendering context.
For each affected parameter, compare all backend templates and identify missing, inconsistent, or intentionally different handling.
When changing a parameter, verify all affected backends, artifact types, and version-specific templates.
List all version-specific templates affected by a fix, using a pattern such asbackend_templates/<backend>/<artifact>*.j2.
Every template variable must be traceable through the pipeline: input, default, rule, mapping, context, and template.
Compare generated output against golden artifacts when available.
Files:
src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2
src/aiconfigurator/generator/rendering/engine.py
📄 CodeRabbit inference engine (.claude/rules/generator/debugging.md)
Trace template variables through
make_worker_context()to verify how values enter the rendering context.Inspect and update
make_worker_context()when template variables are injected or context shape changes.
Files:
src/aiconfigurator/generator/rendering/engine.py
🧠 Learnings (1)
📚 Learning: 2026-05-01T00:39:37.334Z
Learnt from: simone-chen
Repo: ai-dynamo/aiconfigurator PR: 956
File: src/aiconfigurator/sdk/perf_database.py:4144-4151
Timestamp: 2026-05-01T00:39:37.334Z
Learning: In src/aiconfigurator/sdk/**/*.py, preserve upstream metadata for PerformanceResult.source: since PerformanceResult defaults source to "silicon", callers should not force-set result.source (e.g., PerfDatabase._query_silicon_or_hybrid should rely on the default and keep any existing source information rather than overwriting it). Only set source explicitly when you truly intend to change it.
Applied to files:
src/aiconfigurator/sdk/utils.pysrc/aiconfigurator/sdk/s3_models.pysrc/aiconfigurator/sdk/common.py
🪛 ast-grep (0.44.1)
tests/unit/generator/test_vllm_oss_deployment.py
[warning] 43-43: Do not make http calls without encryption
Context: "http://model-express:8001"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
src/aiconfigurator/service/static/index.html
[warning] 799-799: Avoid using the initial state variable in setState
Context: setText("run-status", value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1449-1449: Avoid using the initial state variable in setState
Context: setCode("deployment-selected-output", content)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1458-1458: Avoid using the initial state variable in setState
Context: setCode("records-yaml-output", content)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1493-1493: Avoid using the initial state variable in setState
Context: setSelectedDgd(recommendedArtifact, 推荐方案：${state.selected.mode} Top 1。)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1561-1561: Avoid using the initial state variable in setState
Context: setRecordsDgd(artifact, deployed ? Final deployed DGD: ${artifact.dgd_id} : Generated only. Showing recommended top1 DGD: ${artifact.dgd_id})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1586-1586: Avoid using the initial state variable in setState
Context: setSelectedDgd(payload, note || ${mode} row ${rowIndex}.)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[error] 799-799: React's useState should not be directly called
Context: setText("run-status", value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1449-1449: React's useState should not be directly called
Context: setCode("deployment-selected-output", content)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1450-1450: React's useState should not be directly called
Context: setText("current-dgd-id", artifact && artifact.dgd_id ? artifact.dgd_id : "无")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1458-1458: React's useState should not be directly called
Context: setCode("records-yaml-output", content)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1485-1485: React's useState should not be directly called
Context: setText("current-jid", state.jid || "尚未生成")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1493-1493: React's useState should not be directly called
Context: setSelectedDgd(recommendedArtifact, 推荐方案：${state.selected.mode} Top 1。)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1496-1496: React's useState should not be directly called
Context: setSelectedDgd(null)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1508-1508: React's useState should not be directly called
Context: setStatus("正在加载记录")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1509-1509: React's useState should not be directly called
Context: setError("")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1517-1517: React's useState should not be directly called
Context: setCode("estimate-output", jsonPretty(record))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1530-1530: React's useState should not be directly called
Context: setStatus(deployed ? "已部署" : "已生成")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1545-1545: React's useState should not be directly called
Context: setStatus("正在加载记录")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1546-1546: React's useState should not be directly called
Context: setError("")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1547-1547: React's useState should not be directly called
Context: setRecordsDgd(null, "正在加载记录...")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1561-1561: React's useState should not be directly called
Context: setRecordsDgd(artifact, deployed ? Final deployed DGD: ${artifact.dgd_id} : Generated only. Showing recommended top1 DGD: ${artifact.dgd_id})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1563-1563: React's useState should not be directly called
Context: setStatus(deployed ? "已部署" : "已生成")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1576-1576: React's useState should not be directly called
Context: setStatus("正在查看 DGD")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1577-1577: React's useState should not be directly called
Context: setError("")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1586-1586: React's useState should not be directly called
Context: setSelectedDgd(payload, note || ${mode} row ${rowIndex}.)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1594-1594: React's useState should not be directly called
Context: setStatus("已生成")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1645-1645: React's useState should not be directly called
Context: setStatus("正在估算")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1646-1646: React's useState should not be directly called
Context: setError("")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1647-1647: React's useState should not be directly called
Context: setText("current-jid", "生成中...")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1648-1648: React's useState should not be directly called
Context: setText("current-dgd-id", "无")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1649-1649: React's useState should not be directly called
Context: setSelectedDgd(null, "正在估算，请稍候。")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1662-1662: React's useState should not be directly called
Context: setStatus("已生成")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1665-1665: React's useState should not be directly called
Context: setCode("estimate-output", jsonPretty(payload.estimate))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1678-1678: React's useState should not be directly called
Context: setStatus("正在部署")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1679-1679: React's useState should not be directly called
Context: setError("")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1680-1680: React's useState should not be directly called
Context: setCode("apply-output", 正在部署 ${selectedArtifact.dgd_id || selectedArtifact.mode}...)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1693-1693: React's useState should not be directly called
Context: setStatus("已部署")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1694-1694: React's useState should not be directly called
Context: setCode("apply-output", jsonPretty(payload))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1826-1826: React's useState should not be directly called
Context: setCode("estimate-output", 加载选项失败：${error.message})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
tests/unit/service/test_api.py
[info] 222-222: Do not hardcode temporary file or directory names
Context: "/tmp"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
src/aiconfigurator/service/app.py
[info] 49-49: Do not hardcode temporary file or directory names
Context: "/tmp/aiconfigurator-service/runs.json"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[warning] 75-75: Logging request-derived input unsanitized allows log forging (CRLF injection); strip newlines / encode the value before logging.
Context: logger.exception("Unhandled error", extra={"trace_id": trace_id, "path": request.url.path})
Note: [CWE-117] Improper Output Neutralization for Logs. OWASP A09:2021 Security Logging and Monitoring Failures.
(log-injection-python)
[error] 788-795: Use of unsanitized data to create processes
Context: subprocess.run(
args,
input=stdin,
text=True,
capture_output=True,
timeout=timeout,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 788-795: Command coming from incoming request
Context: subprocess.run(
args,
input=stdin,
text=True,
capture_output=True,
timeout=timeout,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🔇 Additional comments (15)
src/aiconfigurator/sdk/utils.py (1)
1102-1108: LGTM!src/aiconfigurator/generator/rendering/engine.py (1)
34-48: 🎯 Functional CorrectnessNo issue:
--kv-transfer-configis injected by the vLLM templates. This allowlist omission doesn't strip it from disagg K8s output.> Likely an incorrect or invalid review comment.docs/dynamo_deployment_guide.md (1)
510-517: 🗄️ Data Integrity & IntegrationNo change needed:
K8sConfig.oss_*is already in the deployment schema and scoped tovllm.src/aiconfigurator/sdk/common.py (1)
390-402: 🚀 Performance & ScalabilityDrop the caching concern here.
get_support_matrix()andget_configured_s3_model_uris()are already memoized, so this wrapper only rebuilds a small set.> Likely an incorrect or invalid review comment.tests/unit/generator/test_aggregators.py (1)
11-42: LGTM!tests/unit/generator/test_vllm_minimal_k8s_args.py (1)
9-58: LGTM!pyproject.toml (1)
43-43: LGTM!Also applies to: 63-63, 95-95, 109-109
src/aiconfigurator/service/__init__.py (1)
1-7: LGTM!src/aiconfigurator/service/models.py (1)
1-173: LGTM!src/aiconfigurator/service/main.py (1)
1-39: LGTM!src/aiconfigurator/service/app.py (1)
100-266: LGTM!Also applies to: 295-374, 673-711, 980-1046
src/aiconfigurator/service/static/index.html (1)
1-599: LGTM!Also applies to: 644-1835
tests/unit/service/test_api.py (1)
1-351: LGTM!docker/Dockerfile.service (2)
1-25: LGTM!Also applies to: 29-33, 37-60
34-36: 🎯 Functional CorrectnessNo issue:
pyproject.tomlalready defines theserviceextra.> Likely an incorrect or invalid review comment.
| ARG AIC_MODEL_S3_BUCKET=aiplat | ||
| ARG AWS_ENDPOINT_URL=https://oss-s3.haiercash.com | ||
| ARG AWS_DEFAULT_REGION=cn-east-1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Internal company infrastructure identifiers hardcoded as shipped defaults. Both the container image and the demo UI bake in the same internal org's domain (haiercash.com), a private registry mirror path, specific node hostnames, and a secret name — this looks like leftover internal/demo config that shouldn't ship as the default in a public repo.
docker/Dockerfile.service#L26-L28: replacehttps://oss-s3.haiercash.com/cn-east-1/aiplatwith generic placeholder values (e.g.https://s3.example.com,us-east-1) or require them to be supplied at build/run time with no baked-in org-specific default.src/aiconfigurator/service/static/index.html#L605-L643: replace the defaultimage(parties-hub.haiercash.com/...),frontend-node/worker-node(kubeflow-master2),oss-endpoint,oss-region, andoss-secret-namevalues with generic placeholders (or leave blank and require explicit configuration).
📍 Affects 2 files
docker/Dockerfile.service#L26-L28(this comment)src/aiconfigurator/service/static/index.html#L605-L643
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/Dockerfile.service` around lines 26 - 28, Remove internal
infrastructure defaults from docker/Dockerfile.service lines 26-28 by replacing
AIC_MODEL_S3_BUCKET, AWS_ENDPOINT_URL, and AWS_DEFAULT_REGION with generic
placeholders or requiring explicit build/runtime configuration. Also update
src/aiconfigurator/service/static/index.html lines 605-643 to remove the
hardcoded internal image registry, node hostnames, OSS endpoint, region, and
secret name, using generic placeholders or blank values that require explicit
configuration.
| explicit_name = k8s.get("name") | ||
| explicit_name = explicit_name.strip() if isinstance(explicit_name, str) else "" | ||
| name = explicit_name or f"{name_prefix}-{mode_tag}{('-router' if enable_router else '')}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate explicit deployment names as RFC 1123 identifiers.
A value such as UI_Model_1 now reaches metadata.name unchanged and makes the generated DGD invalid. Reject or normalize explicit names and test invalid characters and the 63-character limit. A one-click fix is unsafe because the API must choose between rejection and normalization.
As per coding guidelines, “Generate RFC 1123-compliant DGD names: lowercase alphanumeric characters and hyphens, no longer than 63 characters.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/generator/aggregators.py` around lines 73 - 75, Update the
explicit-name handling near `explicit_name` and `name` to enforce RFC 1123 DGD
naming: lowercase alphanumeric characters and hyphens only, with a maximum
length of 63 characters. Choose and consistently implement either rejecting
invalid explicit names or normalizing them, and add coverage for invalid
characters and overlong names while preserving generated fallback names.
Sources: Coding guidelines, Path instructions
| for name in ("config.json", "tokenizer.json", "tokenizer_config.json", "generation_config.json"): | ||
| try: | ||
| s3.download_file(bucket, f"{prefix}/{name}", os.path.join(meta_dir, name)) | ||
| except Exception: | ||
| if name == "generation_config.json": | ||
| continue | ||
| raise |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 's3_models.py|test_s3_models.py' -x \
rg -n -C3 'tokenizer\.json|tokenizer_config\.json|config\.json' {}Repository: ai-dynamo/aiconfigurator
Length of output: 2052
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the vLLM deployment template around both bootstrap loops.
FILE="src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2"
nl -ba "$FILE" | sed -n '140,180p'
printf '\n---\n'
nl -ba "$FILE" | sed -n '290,325p'
printf '\n=== related S3/model-loading code ===\n'
fd -t f 's3_models.py|*s3*model*|*model*loader*|*model*contract*|*tokenizer*' src | sed -n '1,200p'
printf '\n--- matches ---\n'
rg -n -C3 'tokenizer\.json|tokenizer_config\.json|generation_config\.json|config\.json|download_file|list_s3_models|S3ModelError' src testsRepository: ai-dynamo/aiconfigurator
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2"
printf '=== template excerpt 1 ===\n'
sed -n '150,175p' "$FILE" | cat -n
printf '\n=== template excerpt 2 ===\n'
sed -n '300,320p' "$FILE" | cat -n
printf '\n=== relevant files ===\n'
fd -t f 's3_models.py|test_s3_models.py|*model*loader*|*tokenizer*' src tests
printf '\n=== symbol/text matches ===\n'
rg -n -C3 'tokenizer\.json|tokenizer_config\.json|generation_config\.json|config\.json|download_file|list_s3_models|S3ModelError' src testsRepository: ai-dynamo/aiconfigurator
Length of output: 3132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== candidate files ===\n'
fd -t f 's3_models.py' src tests
fd -t f 'test_s3_models.py' src tests
printf '\n=== s3_models.py ===\n'
sed -n '1,220p' src/aiconfigurator/generator/config/s3_models.py
printf '\n=== tests matching s3_models ===\n'
rg -n -C3 'list_s3_models|load_s3_json|tokenizer\.json|tokenizer_config\.json|generation_config\.json|config\.json' tests src/aiconfigurator/generator/configRepository: ai-dynamo/aiconfigurator
Length of output: 402
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== s3_models.py ===\n'
sed -n '1,260p' src/aiconfigurator/sdk/s3_models.py
printf '\n=== test_s3_models.py ===\n'
sed -n '1,240p' tests/unit/sdk/test_s3_models.py
printf '\n=== tokenizer / config matches in repo ===\n'
rg -n -C2 'tokenizer\.json|tokenizer_config\.json|generation_config\.json|config\.json|list_s3_models|load_s3_json|S3ModelError' src testsRepository: ai-dynamo/aiconfigurator
Length of output: 37537
OSS bootstrap should not require tokenizer files by default
src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2:158-164,307-313 only enforces config.json; the current loop turns tokenizer.json and tokenizer_config.json into hard requirements even though OSS discovery/model loading don’t. Either reject such models earlier or make these downloads optional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2`
around lines 158 - 164, Update the metadata download loop in the vLLM deployment
template so tokenizer.json and tokenizer_config.json are optional, matching the
existing optional handling for generation_config.json; only config.json must
remain required and download failures for it should still propagate. Apply the
same behavior to both affected loop blocks.
Source: Path instructions
| exec python3 -m dynamo.frontend \ | ||
| --model-name {{ served_model_name | tojson }} \ | ||
| --model-path "${META_DIR}" \ | ||
| --router-mode round-robin |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve KV routing in the OSS frontend command.
The explicit round-robin argument conflicts with DYN_ROUTER_MODE=kv when enable_router is true, so OSS deployments cannot use the requested KV router.
Proposed fix
exec python3 -m dynamo.frontend \
--model-name {{ served_model_name | tojson }} \
--model-path "${META_DIR}" \
- --router-mode round-robin
+ --router-mode {{ ('kv' if enable_router else 'round-robin') | tojson }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exec python3 -m dynamo.frontend \ | |
| --model-name {{ served_model_name | tojson }} \ | |
| --model-path "${META_DIR}" \ | |
| --router-mode round-robin | |
| exec python3 -m dynamo.frontend \ | |
| --model-name {{ served_model_name | tojson }} \ | |
| --model-path "${META_DIR}" \ | |
| --router-mode {{ ('kv' if enable_router else 'round-robin') | tojson }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/aiconfigurator/generator/config/backend_templates/vllm/k8s_deploy.yaml.j2`
around lines 315 - 318, Remove the explicit --router-mode round-robin argument
from the OSS frontend command in the vllm deployment template, allowing
DYN_ROUTER_MODE=kv to control routing when enable_router is enabled. Preserve
the existing model-name and model-path arguments.
Source: Path instructions
| default: >- | ||
| ServiceConfig.model_name or | ||
| (ServiceConfig.model_path.split('/')[-2] if ServiceConfig.model_path.startswith('s3://') else | ||
| (ServiceConfig.served_model_path or ServiceConfig.model_path)) | ||
| - key: ServiceConfig.model_name | ||
| required: false | ||
| default: "ServiceConfig.served_model_name" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the circular model-name defaults.
served_model_name reads model_name, while model_name defaults back to served_model_name. Make one field authoritative and derive the other in a single direction; otherwise schema evaluation order can leave both unset.
As per coding guidelines, “Avoid circular default references between parameters because the schema loader may produce None for both.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/generator/config/deployment_config.yaml` around lines 23 -
29, Remove the circular defaults between ServiceConfig.model_name and
ServiceConfig.served_model_name in the deployment configuration. Make one field
authoritative and have the other derive from it in only one direction, ensuring
schema evaluation cannot leave both values unset.
Source: Coding guidelines
| def _cache_transient_run(jid: str, request: ArtifactRequest, payload: dict[str, Any]) -> None: | ||
| _TRANSIENT_RUNS[jid] = { | ||
| "jid": jid, | ||
| "created_at": datetime.now(timezone.utc).isoformat(), | ||
| "request": _normalize_obj(request.model_dump(exclude_none=True)), | ||
| "payload": _normalize_obj(payload), | ||
| } | ||
|
|
||
|
|
||
| def _save_run_record(jid: str, request: ArtifactRequest, payload: dict[str, Any]) -> None: | ||
| record = { | ||
| "jid": jid, | ||
| "created_at": datetime.now(timezone.utc).isoformat(), | ||
| "request": _normalize_obj(request.model_dump(exclude_none=True)), | ||
| "payload": _normalize_obj(payload), | ||
| } | ||
| _save_record(jid, record) | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Credential (hf_token) is persisted to disk and echoed back by GET /api/v1/runs/{jid}.
_cache_transient_run/_save_run_record dump the raw ArtifactRequest (including generator_overrides.service_config.hf_token when supplied) into the run store. get_run (Line 217-222) later returns that record verbatim, so any caller who learns a jid receives the Hugging Face token in plaintext, and it also lands unredacted in the run-store JSON file on disk.
🔒 Redact secrets before persisting/returning run records
+def _redact_secrets(data: dict[str, Any]) -> dict[str, Any]:
+ redacted = copy.deepcopy(data)
+ service_cfg = (redacted.get("generator_overrides") or {}).get("service_config")
+ if isinstance(service_cfg, dict) and service_cfg.get("hf_token"):
+ service_cfg["hf_token"] = "***redacted***"
+ return redacted
+
+
def _cache_transient_run(jid: str, request: ArtifactRequest, payload: dict[str, Any]) -> None:
_TRANSIENT_RUNS[jid] = {
"jid": jid,
"created_at": datetime.now(timezone.utc).isoformat(),
- "request": _normalize_obj(request.model_dump(exclude_none=True)),
+ "request": _redact_secrets(_normalize_obj(request.model_dump(exclude_none=True))),
"payload": _normalize_obj(payload),
}Apply the same wrapper in _save_run_record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/service/app.py` around lines 486 - 503, Redact sensitive
credentials before storing run records: update both _cache_transient_run and
_save_run_record to pass request data through the existing secret-redaction
wrapper before normalization and persistence. Ensure hf_token and any other
supported secrets are removed or masked consistently, so get_run cannot return
plaintext credentials while preserving the rest of the request record.
| def _apply_k8s_deployment(request: ApplyDeploymentRequest) -> dict[str, Any]: | ||
| if shutil.which("kubectl") is None: | ||
| raise APIError( | ||
| 503, | ||
| "kubectl_not_available", | ||
| "kubectl is not available in the service runtime.", | ||
| {"hint": "Install kubectl in the AIC service image and grant its ServiceAccount apply/get permissions."}, | ||
| ) | ||
|
|
||
| resources = _parse_k8s_resources(request.content, request.namespace) | ||
| apply_result = _run_kubectl( | ||
| ["kubectl", "apply", "-f", "-"], | ||
| stdin=request.content, | ||
| timeout=request.timeout_seconds, | ||
| ) | ||
| health = _check_k8s_health(resources, request.timeout_seconds) | ||
| payload = { | ||
| "applied": apply_result.returncode == 0, | ||
| "stdout": apply_result.stdout, | ||
| "stderr": apply_result.stderr, | ||
| "resources": resources, | ||
| "health": health, | ||
| "healthy": all(item.get("exists") for item in health), | ||
| } | ||
| _mark_deployed_dgd(request, payload) | ||
| return payload | ||
|
|
||
|
|
||
| def _delete_k8s_deployment(request: DeleteDeploymentRequest) -> dict[str, Any]: | ||
| if shutil.which("kubectl") is None: | ||
| raise APIError( | ||
| 503, | ||
| "kubectl_not_available", | ||
| "kubectl is not available in the service runtime.", | ||
| {"hint": "Install kubectl in the AIC service image and grant its ServiceAccount delete/get permissions."}, | ||
| ) | ||
|
|
||
| resources = _parse_k8s_resources(request.content, request.namespace) | ||
| delete_result = _run_kubectl( | ||
| ["kubectl", "delete", "-f", "-", "--ignore-not-found=true"], | ||
| stdin=request.content, | ||
| timeout=request.timeout_seconds, | ||
| ) | ||
| payload = { | ||
| "deleted": delete_result.returncode == 0, | ||
| "stdout": delete_result.stdout, | ||
| "stderr": delete_result.stderr, | ||
| "resources": resources, | ||
| } | ||
| _mark_deleted_dgd(request, payload) | ||
| return payload |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file first, then inspect the route and helper definitions around the cited lines.
ast-grep outline src/aiconfigurator/service/app.py --view expanded || true
printf '\n--- lines 650-900 ---\n'
sed -n '650,900p' src/aiconfigurator/service/app.pyRepository: ai-dynamo/aiconfigurator
Length of output: 13815
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect request models and any auth-related middleware/decorators used by the app.
rg -n "create_app|APIRouter|Depends|OAuth|auth|authorize|permission|kubectl|get rollout|rollout status|_parse_k8s_resources|K8sConfigInput|ApplyDeploymentRequest|DeleteDeploymentRequest" src/aiconfigurator/service/app.py src/aiconfigurator -SRepository: ai-dynamo/aiconfigurator
Length of output: 8206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the manifest parser or request model constrains Kubernetes kinds/namespaces.
sed -n '1,220p' src/aiconfigurator/service/app.pyRepository: ai-dynamo/aiconfigurator
Length of output: 8802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
path = pathlib.Path('src/aiconfigurator/service/app.py')
text = path.read_text()
for needle in ['def create_app', 'def _parse_k8s_resources', 'ApplyDeploymentRequest', 'DeleteDeploymentRequest', 'K8sConfigInput', 'kubectl get', 'rollout status']:
print(f'## {needle}')
for m in re.finditer(re.escape(needle), text):
start = max(0, m.start()-400)
end = min(len(text), m.end()+800)
print(text[start:end])
print('\n---\n')
break
PYRepository: ai-dynamo/aiconfigurator
Length of output: 5223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the request models for validation on manifest content, kind, name, namespace, and timeout.
ast-grep outline src/aiconfigurator/service/models.py --view expanded || true
printf '\n--- models excerpt ---\n'
sed -n '1,260p' src/aiconfigurator/service/models.pyRepository: ai-dynamo/aiconfigurator
Length of output: 12109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any authn/authz middleware, dependencies, API keys, or security schemes in the service package.
rg -n "Depends\(|APIKey|Authorization|authn|authz|oauth|JWT|Bearer|security|permission|role|middleware" src/aiconfigurator/service -SRepository: ai-dynamo/aiconfigurator
Length of output: 1079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('src/aiconfigurator/service/models.py').read_text()
for needle in ['class K8sConfigInput', 'class ApplyDeploymentRequest', 'class DeleteDeploymentRequest']:
i = text.find(needle)
if i != -1:
print(f'## {needle}')
print(text[i:i+2200])
print('\n---\n')
PYRepository: ai-dynamo/aiconfigurator
Length of output: 3080
Restrict the Kubernetes deploy routes and sanitize resource identifiers. src/aiconfigurator/service/app.py:713-859 accepts raw manifest content and runs kubectl apply/delete with the service account’s RBAC, with no in-app authz gate. The follow-up kubectl get/rollout status calls also trust manifest-derived kind/name/namespace; reject flag-like values (for example --help) before building the argv.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/service/app.py` around lines 713 - 763, Add an in-app
authorization check at the entry points for _apply_k8s_deployment and
_delete_k8s_deployment before invoking kubectl, using the service’s existing
authz mechanism. Validate the resources returned by _parse_k8s_resources,
including kind, name, and namespace, and reject flag-like or otherwise invalid
identifiers before constructing any kubectl argv. Ensure the same validated
identifiers are used by _check_k8s_health and related rollout/get commands.
| if kind.lower() in {"deployment", "statefulset", "daemonset"}: | ||
| rollout = _run_kubectl( | ||
| [ | ||
| "kubectl", | ||
| "rollout", | ||
| "status", | ||
| f"{kind}/{name}", | ||
| "-n", | ||
| namespace, | ||
| f"--timeout={timeout_seconds}s", | ||
| ], | ||
| timeout=timeout_seconds + 5, | ||
| ) | ||
| return {**base, "exists": True, "ready": True, "message": rollout.stdout.strip()} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A slow/failed rollout aborts the whole apply response, losing the fact that kubectl apply already succeeded.
For Deployment/StatefulSet/DaemonSet, _run_kubectl's rollout status call raises APIError on non-zero exit (including a legitimate "not ready within timeout" exit code). That exception propagates out of _check_k8s_health and _apply_k8s_deployment before payload/_mark_deployed_dgd are ever built, so a merely-slow-to-roll-out Deployment is reported as a hard 502 failure and the deployment record is never marked deployed — even though the resources were created.
🩹 Turn rollout failures into a health entry instead of raising
if kind.lower() in {"deployment", "statefulset", "daemonset"}:
- rollout = _run_kubectl(
- [
- "kubectl",
- "rollout",
- "status",
- f"{kind}/{name}",
- "-n",
- namespace,
- f"--timeout={timeout_seconds}s",
- ],
- timeout=timeout_seconds + 5,
- )
- return {**base, "exists": True, "ready": True, "message": rollout.stdout.strip()}
+ try:
+ rollout = _run_kubectl(
+ ["kubectl", "rollout", "status", f"{kind}/{name}", "-n", namespace, f"--timeout={timeout_seconds}s"],
+ timeout=timeout_seconds + 5,
+ )
+ except APIError as exc:
+ return {**base, "exists": True, "ready": False, "message": str(exc)}
+ return {**base, "exists": True, "ready": True, "message": rollout.stdout.strip()}This path is also untested — test_delete_k8s_deployment_removes_deployed_record only exercises the success case via a generic mock. As per path instructions, tests should cover the changed behavior, not only the happy path.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if kind.lower() in {"deployment", "statefulset", "daemonset"}: | |
| rollout = _run_kubectl( | |
| [ | |
| "kubectl", | |
| "rollout", | |
| "status", | |
| f"{kind}/{name}", | |
| "-n", | |
| namespace, | |
| f"--timeout={timeout_seconds}s", | |
| ], | |
| timeout=timeout_seconds + 5, | |
| ) | |
| return {**base, "exists": True, "ready": True, "message": rollout.stdout.strip()} | |
| if kind.lower() in {"deployment", "statefulset", "daemonset"}: | |
| try: | |
| rollout = _run_kubectl( | |
| ["kubectl", "rollout", "status", f"{kind}/{name}", "-n", namespace, f"--timeout={timeout_seconds}s"], | |
| timeout=timeout_seconds + 5, | |
| ) | |
| except APIError as exc: | |
| return {**base, "exists": True, "ready": False, "message": str(exc)} | |
| return {**base, "exists": True, "ready": True, "message": rollout.stdout.strip()} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/service/app.py` around lines 824 - 837, Update the
deployment/statefulset/daemonset branch in _check_k8s_health to catch APIError
from the rollout-status _run_kubectl call and return an unhealthy health entry
containing the rollout failure details instead of propagating the exception.
Preserve the successful response for completed rollouts so _apply_k8s_deployment
can still build its payload and mark the deployment record as deployed, and add
coverage for the failed or timed-out rollout path.
Source: Path instructions
| function deriveServedModelName(modelPath) { | ||
| const cleaned = modelPath.trim().replace(/[\\/]+$/, ""); | ||
| if (!cleaned) { | ||
| return ""; | ||
| } | ||
| const segments = cleaned.split(/[\\/]/).filter(Boolean); | ||
| const modelSegment = cleaned.startsWith("s3://") && segments.length >= 2 | ||
| ? segments.at(-2) | ||
| : segments.at(-1) || cleaned; | ||
| return modelSegment.replace(/[^A-Za-z0-9._-]+/g, "-").toLowerCase(); | ||
| } | ||
|
|
||
| function deriveDeploymentName(servedModelName) { | ||
| const cleaned = servedModelName.trim().replace(/[^A-Za-z0-9._-]+/g, "-").toLowerCase(); | ||
| return cleaned ? `dynamo-${cleaned}` : "dynamo"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Served-model/deployment-name derivation can still produce non-RFC1123 names.
The sanitizer regex [^A-Za-z0-9._-]+ keeps . and _ and applies no length cap, so a model path containing an underscore (e.g. Qwen3_8B) or a long path produces a k8s resource name that's not lowercase-alphanumeric-and-hyphens-only and can exceed 63 chars.
🏷️ Strip to RFC1123-safe characters and cap length
function deriveServedModelName(modelPath) {
const cleaned = modelPath.trim().replace(/[\\/]+$/, "");
if (!cleaned) {
return "";
}
const segments = cleaned.split(/[\\/]/).filter(Boolean);
const modelSegment = cleaned.startsWith("s3://") && segments.length >= 2
? segments.at(-2)
: segments.at(-1) || cleaned;
- return modelSegment.replace(/[^A-Za-z0-9._-]+/g, "-").toLowerCase();
+ return modelSegment
+ .replace(/[^A-Za-z0-9-]+/g, "-")
+ .toLowerCase()
+ .replace(/^-+|-+$/g, "")
+ .slice(0, 63);
}
function deriveDeploymentName(servedModelName) {
- const cleaned = servedModelName.trim().replace(/[^A-Za-z0-9._-]+/g, "-").toLowerCase();
- return cleaned ? `dynamo-${cleaned}` : "dynamo";
+ const cleaned = servedModelName.trim().replace(/[^A-Za-z0-9-]+/g, "-").toLowerCase().replace(/^-+|-+$/g, "");
+ return (cleaned ? `dynamo-${cleaned}` : "dynamo").slice(0, 63);
}As per path instructions, "Generate RFC 1123-compliant DGD names: lowercase alphanumeric characters and hyphens, no longer than 63 characters."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function deriveServedModelName(modelPath) { | |
| const cleaned = modelPath.trim().replace(/[\\/]+$/, ""); | |
| if (!cleaned) { | |
| return ""; | |
| } | |
| const segments = cleaned.split(/[\\/]/).filter(Boolean); | |
| const modelSegment = cleaned.startsWith("s3://") && segments.length >= 2 | |
| ? segments.at(-2) | |
| : segments.at(-1) || cleaned; | |
| return modelSegment.replace(/[^A-Za-z0-9._-]+/g, "-").toLowerCase(); | |
| } | |
| function deriveDeploymentName(servedModelName) { | |
| const cleaned = servedModelName.trim().replace(/[^A-Za-z0-9._-]+/g, "-").toLowerCase(); | |
| return cleaned ? `dynamo-${cleaned}` : "dynamo"; | |
| } | |
| function deriveServedModelName(modelPath) { | |
| const cleaned = modelPath.trim().replace(/[\\/]+$/, ""); | |
| if (!cleaned) { | |
| return ""; | |
| } | |
| const segments = cleaned.split(/[\\/]/).filter(Boolean); | |
| const modelSegment = cleaned.startsWith("s3://") && segments.length >= 2 | |
| ? segments.at(-2) | |
| : segments.at(-1) || cleaned; | |
| return modelSegment | |
| .replace(/[^A-Za-z0-9-]+/g, "-") | |
| .toLowerCase() | |
| .replace(/^-+|-+$/g, "") | |
| .slice(0, 63); | |
| } | |
| function deriveDeploymentName(servedModelName) { | |
| const cleaned = servedModelName.trim().replace(/[^A-Za-z0-9-]+/g, "-").toLowerCase().replace(/^-+|-+$/g, ""); | |
| return (cleaned ? `dynamo-${cleaned}` : "dynamo").slice(0, 63); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiconfigurator/service/static/index.html` around lines 827 - 842, Update
deriveServedModelName and deriveDeploymentName to generate RFC1123-compliant
names: lowercase letters, digits, and hyphens only, with each resulting name
capped at 63 characters. Replace the current sanitizer behavior that preserves
dots and underscores, and ensure truncation does not leave invalid leading or
trailing hyphens.
Source: Path instructions
| @pytest.mark.unit | ||
| def test_vllm_oss_deployment_uses_model_express_and_secret_refs(): | ||
| template_dir = Path("src/aiconfigurator/generator/config/backend_templates/vllm") | ||
| template = Environment(loader=FileSystemLoader(template_dir)).get_template("k8s_deploy.yaml.j2") | ||
| rendered = template.render( | ||
| working_dir="/workspace/examples/backends/vllm", | ||
| K8sConfig={ | ||
| "oss_enabled": True, | ||
| "oss_endpoint_url": "https://oss-s3.haiercash.com", | ||
| "oss_region": "cn-east-1", | ||
| "oss_secret_name": "oss-s3-secret", | ||
| "oss_model_express_url": "http://model-express:8001", | ||
| "oss_streamer_concurrency": 4, | ||
| "k8s_namespace": "aic-system", | ||
| "k8s_image": "example/vllm:modelexpress", | ||
| "frontend_node_selector": {"kubernetes.io/hostname": "master2"}, | ||
| "worker_node_selector": {"kubernetes.io/hostname": "master2"}, | ||
| }, | ||
| ServiceConfig={ | ||
| "model_path": "s3://aiplat/ai-lab/qwen3-0.6b/2026-07-01", | ||
| "served_model_name": "qwen3-0.6b", | ||
| }, | ||
| DynConfig={"mode": "agg", "enable_router": False}, | ||
| name="dynamo-qwen3-oss", | ||
| frontend_replicas=1, | ||
| agg_workers=1, | ||
| agg_gpu=1, | ||
| agg_cli_args_list=["--tensor-parallel-size", "1", "--max-model-len", "4096"], | ||
| ) | ||
|
|
||
| deployment = yaml.safe_load(rendered) | ||
| worker = deployment["spec"]["services"]["VllmWorker"]["extraPodSpec"]["mainContainer"] | ||
| script = worker["args"][0] | ||
| env = {item["name"]: item for item in worker["env"]} | ||
|
|
||
| assert env["MX_MODEL_URI"]["value"] == "s3://aiplat/ai-lab/qwen3-0.6b/2026-07-01" | ||
| assert env["AWS_ENDPOINT_URL"]["value"] == "https://oss-s3.haiercash.com" | ||
| assert env["AWS_ACCESS_KEY_ID"]["valueFrom"]["secretKeyRef"]["name"] == "oss-s3-secret" | ||
| assert "--load-format modelexpress" in script | ||
| assert 'worker_cmd+=( "--tensor-parallel-size" )' in script | ||
| assert "config.json" in script |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 \
'test_vllm_oss|generate_backend_artifacts|golden|generator_validator|kubectl.*dry-run' \
tests src/aiconfigurator/generatorRepository: ai-dynamo/aiconfigurator
Length of output: 13488
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## test_vllm_oss_deployment.py\n'
sed -n '1,220p' tests/unit/generator/test_vllm_oss_deployment.py
printf '\n## golden backend contracts around vllm\n'
sed -n '1,260p' tests/golden/generator/test_release_1_2_0_backend_contracts.py
printf '\n## generator api render path\n'
sed -n '240,330p' src/aiconfigurator/generator/api.py
printf '\n## search for vllm OSS / model express / golden references\n'
rg -n -C2 'oss_enabled|model_express|OSS|vllm_oss|k8s_deploy.yaml|generate_backend_artifacts|generator_validator|dry-run' tests src toolsRepository: ai-dynamo/aiconfigurator
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## OSS-specific references in tests'
rg -n -C3 'oss_enabled|model_express|oss_endpoint_url|oss_secret_name|oss_streamer_concurrency|load-format modelexpress' tests
echo
echo '## vllm-related golden/integration tests'
rg -n -C2 'vllm|backend_artifacts|k8s_deploy|run.sh|cli_args_agg|extra_engine_args' tests/golden tests/integration tests/unit/generator tests/unit/generator_validator
echo
echo '## file list for generator tests'
git ls-files 'tests/**' | rg 'vllm|generator|golden|integration'Repository: ai-dynamo/aiconfigurator
Length of output: 50380
Add OSS coverage through the generator pipeline
This test only renders k8s_deploy.yaml.j2 directly. Add a generator-level case for an s3:// model path that exercises apply_defaults + generate_backend_artifacts and checks the emitted k8s_deploy.yaml/golden artifact for the OSS and ModelExpress fields.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 43-43: Do not make http calls without encryption
Context: "http://model-express:8001"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/generator/test_vllm_oss_deployment.py` around lines 33 - 73, Add a
generator-level test alongside
test_vllm_oss_deployment_uses_model_express_and_secret_refs that uses an s3://
model path and runs apply_defaults followed by generate_backend_artifacts.
Validate the emitted k8s_deploy.yaml or golden artifact contains the expected
OSS endpoint, secret reference, model URI, and ModelExpress settings instead of
only rendering k8s_deploy.yaml.j2 directly.
Sources: Coding guidelines, Path instructions
1 participant
What changed
Why
The service previously assumed local or Hugging Face model paths. This adds an OSS-backed workflow for environments where model artifacts are stored in an S3-compatible object store and streamed into vLLM through ModelExpress.
Impact
Users can browse models from OSS, estimate configurations, and generate/deploy vLLM manifests without embedding credentials or mounting a local model directory. Existing non-OSS generator behavior remains the fallback.
Validation
TMPDIR=/tmp .venv/bin/pytest -s -q tests/unit/generator/test_aggregators.py tests/unit/generator/test_vllm_oss_deployment.py tests/unit/sdk/test_s3_models.py tests/unit/service(24 passed)ruff checkfor changed Python modules and testsgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation