Add DeepSeek V4 serving profiling skill - #125
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a documented eight-NPU DSV4 serving profiling skill with scripts to launch a server, submit a fixed completion request, analyze serving and host STRACE data, and render collapsed or detailed eight-lane trace artifacts. ChangesDSV4 serving profiling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant run_profile.sh
participant run_profile.py
participant pypto-serving
participant analyze_profile.py
participant render_8lane.py
Operator->>run_profile.sh: provide model and device options
run_profile.sh->>run_profile.py: configure and execute profile
run_profile.py->>pypto-serving: start, poll health, and submit completion
pypto-serving-->>run_profile.py: response and profiling logs
run_profile.sh->>analyze_profile.py: analyze artifact directory
analyze_profile.py-->>run_profile.sh: profile summary and swimlane JSON
run_profile.sh->>render_8lane.py: render eight-lane traces
render_8lane.py-->>Operator: collapsed and detailed trace JSON
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
.agents/skills/profile-dsv4-serving-strace/scripts/render_8lane.py (1)
23-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: the
stepstored in thedecodetable is dead.Line 40 discards the middle element and returns the locally computed
step, which is the same value. Two-tuples make the intent clearer.♻️ Proposed refactor
decode = { - 0: ("decode.main", step, "good"), - 1: ("decode.main.lm_head", step, "rail_animation"), - 2: ("decode.mtp", step, "cq_build_running"), - 3: ("decode.mtp.lm_head", step, "rail_idle"), + 0: ("decode.main", "good"), + 1: ("decode.main.lm_head", "rail_animation"), + 2: ("decode.mtp", "cq_build_running"), + 3: ("decode.mtp.lm_head", "rail_idle"), } - label, _, color = decode[phase] + label, color = decode[phase] return label, step, color🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/render_8lane.py around lines 23 - 41, Update callable_label’s decode mapping to store only the label and color, since the mapped step value is discarded; unpack the two-element entries and continue returning the locally computed step unchanged..agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py (2)
101-120: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
process.wait(timeout=10)in the escalation tail can mask the original failure.
stop_serverruns from afinallyblock; if theSIGKILLreap still times out, theTimeoutExpiredreplaces the real exception from the profiling run. Wrapping the final wait keeps the underlying error visible.♻️ Proposed refactor
try: os.killpg(process.pid, signal.SIGKILL) except OSError: pass - process.wait(timeout=10) + try: + process.wait(timeout=10) + except subprocess.TimeoutExpired: + print(f"warning: server pid {process.pid} did not exit after SIGKILL", flush=True)🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py around lines 101 - 120, Update stop_server so the final process.wait(timeout=10) after SIGKILL cannot replace an exception already propagating from the profiling run; catch and suppress TimeoutExpired (while preserving existing OSError handling), allowing the original failure to remain visible.
227-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRe-parsing argv in the failure handler is fragile.
parse_args()runs a second time purely to recover the artifact path, and the nestedexcept BaseException: passswallows any reason it failed. Resolving the log path once up front and reusing it keeps the tail-printing path deterministic (also addresses the RuffS110/BLE001hints).♻️ Proposed refactor
if __name__ == "__main__": + _server_log: Path | None = None try: - raise SystemExit(main()) - except BaseException: - artifact = None - try: - artifact = parse_args().artifact_dir.resolve() / "server.log" - except BaseException: - pass - if artifact is not None: - print_log_tail(artifact) + _server_log = parse_args().artifact_dir.resolve() / "server.log" + except SystemExit: + raise + try: + raise SystemExit(main()) + except BaseException: + if _server_log is not None: + print_log_tail(_server_log) raise🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py around lines 227 - 238, Refactor the __main__ failure path to parse arguments and resolve the artifact directory once before invoking main(), retain the resulting server.log path for exception handling, and reuse it in print_log_tail. Remove the second parse_args() call and the nested broad exception suppression while preserving the existing exception re-raise behavior.Source: Linters/SAST tools
.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.sh (1)
125-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider printing the artifact directory even when post-processing fails.
With
set -e, a failure inanalyze_profile.pyorrender_8lane.pyaborts before Line 134, so the operator loses the path to the (expensive) run they just captured. Atrapmakes the location discoverable on any exit.♻️ Proposed refactor
+trap 'echo "Artifacts: $ARTIFACT_DIR"' EXIT + "$PYTHON_BIN" "$SCRIPT_DIR/analyze_profile.py" "$ARTIFACT_DIR" \ --run-id "$RUN_ID" --expected-tokens "$MAX_TOKENS" "$PYTHON_BIN" "$SCRIPT_DIR/render_8lane.py" \ "$ARTIFACT_DIR/simpler-swimlane.json" "$ARTIFACT_DIR/server.log" \ "$ARTIFACT_DIR/strace-8lane.json" "$PYTHON_BIN" "$SCRIPT_DIR/render_8lane.py" \ "$ARTIFACT_DIR/simpler-swimlane.json" "$ARTIFACT_DIR/server.log" \ "$ARTIFACT_DIR/strace-8lane-host-clock.json" --detailed --host-only - -echo "Artifacts: $ARTIFACT_DIR"🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.sh around lines 125 - 134, Update the top-level flow in run_profile.sh to register an EXIT trap that always prints the ARTIFACT_DIR, including when analyze_profile.py or either render_8lane.py invocation fails under set -e. Avoid relying solely on the final echo, and ensure the directory is reported once on normal and error exits..agents/skills/profile-dsv4-serving-strace/scripts/launch_server.py (1)
18-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParse and validate
PYPTO_DSV4_TOTAL_KV_PAGESbefore patching the runtime builder.The current
int(...)is evaluated inside_build_runtime_config, after CLI parsing and engine/device setup; invalid input raisesValueErrorlater than needed. Parse it inmain()and reject non-positive values beforemain()delegates to the CLI path. Also keep the note that this skill depends on the privatecli_main._build_runtime_config; if there is no supported CLI/env override, this keeps the skill aligned with that private hook.🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/launch_server.py around lines 18 - 30, In main, parse PYPTO_DSV4_TOTAL_KV_PAGES once before patching cli_main._build_runtime_config, reject non-positive values immediately, and reuse the validated integer in build_runtime_config instead of reading the environment there. Preserve the existing dataclasses.replace override and document that this skill intentionally depends on the private _build_runtime_config hook when no supported CLI/environment override exists..agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py (1)
144-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid relying on
_round_metricspositional tuple indices.
rank_phase_row()expects_round_metrics(...)to return a stable 3-tuple and indexes0for host us and2for effective us. Since this dependency is imported as a private symbol, an upstream reorder can change host/effective values without failing. Localize the ordering, e.g. withHOST_US_INDEX = 0andEFFECTIVE_US_INDEX = 2, or preferably use a named accessor if one exists.🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py around lines 144 - 152, Update metric_us and rank_phase_row to avoid unexplained positional access to _round_metrics tuple values. Define and use named constants such as HOST_US_INDEX and EFFECTIVE_US_INDEX, or an existing named accessor if available, so host and effective microsecond metrics remain mapped correctly if the tuple ordering changes.
🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py:
- Around line 320-322: Validate main_serving and mtp_serving after retrieving
steady_after_first in the profile analysis flow, before their mean fields are
dereferenced. Raise a clear, descriptive error when either value is None, while
preserving the existing calculations for valid serving summaries.
- Around line 220-224: Update the request_ms extraction in the profile analysis
flow to use next with a None default, then validate the result and raise a
descriptive RuntimeError when no http.completions span exists. Preserve the
existing duration conversion for matching serving_events.
In @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py:
- Around line 216-219: Make artifact encoding explicit as UTF-8: update both
write_text calls for completion-response.json and completion.txt in
.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py (lines
216-219), and both read_text calls for server.log and serving-trace/trace.json
in .agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py (lines
85-86), passing encoding="utf-8" to each call.
- Around line 211-215: Update the usage extraction in the response validation
flow to handle an explicit null value, using an empty mapping fallback when
response.get("usage") returns None. Preserve the existing completion-token
assertion and diagnostic for populated usage objects.
In @.agents/skills/profile-dsv4-serving-strace/SKILL.md:
- Around line 13-15: Update run_profile.sh to fail closed when allocation
metadata is unavailable instead of defaulting to device IDs 0..7. Require an
explicit --devices value in that case, while preserving scheduler-provided
allocation handling and the existing eight-NPU workflow.
- Around line 75-80: Update the validation contract and reprocessing flow around
run_profile.sh so the expected token count follows the selected --max-tokens
value instead of hardcoding 20. Either remove/restrict the override or propagate
the selected limit into the success check and reprocessing --expected-tokens
argument, preserving code-0 validation.
---
Nitpick comments:
In @.agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py:
- Around line 144-152: Update metric_us and rank_phase_row to avoid unexplained
positional access to _round_metrics tuple values. Define and use named constants
such as HOST_US_INDEX and EFFECTIVE_US_INDEX, or an existing named accessor if
available, so host and effective microsecond metrics remain mapped correctly if
the tuple ordering changes.
In @.agents/skills/profile-dsv4-serving-strace/scripts/launch_server.py:
- Around line 18-30: In main, parse PYPTO_DSV4_TOTAL_KV_PAGES once before
patching cli_main._build_runtime_config, reject non-positive values immediately,
and reuse the validated integer in build_runtime_config instead of reading the
environment there. Preserve the existing dataclasses.replace override and
document that this skill intentionally depends on the private
_build_runtime_config hook when no supported CLI/environment override exists.
In @.agents/skills/profile-dsv4-serving-strace/scripts/render_8lane.py:
- Around line 23-41: Update callable_label’s decode mapping to store only the
label and color, since the mapped step value is discarded; unpack the
two-element entries and continue returning the locally computed step unchanged.
In @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py:
- Around line 101-120: Update stop_server so the final process.wait(timeout=10)
after SIGKILL cannot replace an exception already propagating from the profiling
run; catch and suppress TimeoutExpired (while preserving existing OSError
handling), allowing the original failure to remain visible.
- Around line 227-238: Refactor the __main__ failure path to parse arguments and
resolve the artifact directory once before invoking main(), retain the resulting
server.log path for exception handling, and reuse it in print_log_tail. Remove
the second parse_args() call and the nested broad exception suppression while
preserving the existing exception re-raise behavior.
In @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.sh:
- Around line 125-134: Update the top-level flow in run_profile.sh to register
an EXIT trap that always prints the ARTIFACT_DIR, including when
analyze_profile.py or either render_8lane.py invocation fails under set -e.
Avoid relying solely on the final echo, and ensure the directory is reported
once on normal and error exits.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8abeeb80-77e4-4a96-92f4-3586a5dfe7a9
📒 Files selected for processing (7)
.agents/skills/profile-dsv4-serving-strace/SKILL.md.agents/skills/profile-dsv4-serving-strace/agents/openai.yaml.agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py.agents/skills/profile-dsv4-serving-strace/scripts/launch_server.py.agents/skills/profile-dsv4-serving-strace/scripts/render_8lane.py.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.sh
| request_ms = next( | ||
| event["dur"] / 1000.0 | ||
| for event in serving_events | ||
| if event.get("ph") == "X" and event.get("name") == "http.completions" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Bare next() raises StopIteration instead of a diagnostic.
Every other validation gate in this script raises a descriptive RuntimeError. A trace missing the http.completions span (e.g. a streaming or aborted request) surfaces here as an opaque StopIteration.
🛡️ Proposed fix
request_ms = next(
event["dur"] / 1000.0
for event in serving_events
if event.get("ph") == "X" and event.get("name") == "http.completions"
- )
+ ),
+ None,
+ )
+ if request_ms is None:
+ raise RuntimeError("serving trace has no 'http.completions' span")(adjust to next((... generator ...), None) — the generator needs wrapping parens.)
🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py around
lines 220 - 224, Update the request_ms extraction in the profile analysis flow
to use next with a None default, then validate the result and raise a
descriptive RuntimeError when no http.completions span exists. Preserve the
existing duration conversion for matching serving_events.
| prefill = summary["simpler"]["prefill_critical"] | ||
| main_serving = summary["serving_kernel_ms"]["deepseek_v4_decode"]["steady_after_first"] | ||
| mtp_serving = summary["serving_kernel_ms"]["deepseek_v4_mtp_decode"]["steady_after_first"] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
steady_after_first may be None here.
kernel_summary returns None for steady_after_first when a kernel has a single sample (Line 250), and Lines 378-379 dereference main_serving["mean"] / mtp_serving["mean"] unconditionally. The decode-row guard at Line 242 constrains STRACE invocations, not serving kernel span counts, so the coupling is implicit. An explicit check would fail with a clear message instead of TypeError: 'NoneType' object is not subscriptable.
🛡️ Proposed fix
main_serving = summary["serving_kernel_ms"]["deepseek_v4_decode"]["steady_after_first"]
mtp_serving = summary["serving_kernel_ms"]["deepseek_v4_mtp_decode"]["steady_after_first"]
+ if main_serving is None or mtp_serving is None:
+ raise RuntimeError("need more than one decode kernel span for steady statistics")📝 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.
| prefill = summary["simpler"]["prefill_critical"] | |
| main_serving = summary["serving_kernel_ms"]["deepseek_v4_decode"]["steady_after_first"] | |
| mtp_serving = summary["serving_kernel_ms"]["deepseek_v4_mtp_decode"]["steady_after_first"] | |
| prefill = summary["simpler"]["prefill_critical"] | |
| main_serving = summary["serving_kernel_ms"]["deepseek_v4_decode"]["steady_after_first"] | |
| mtp_serving = summary["serving_kernel_ms"]["deepseek_v4_mtp_decode"]["steady_after_first"] | |
| if main_serving is None or mtp_serving is None: | |
| raise RuntimeError("need more than one decode kernel span for steady statistics") |
🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py around
lines 320 - 322, Validate main_serving and mtp_serving after retrieving
steady_after_first in the profile analysis flow, before their mean fields are
dereferenced. Raise a clear, descriptive error when either value is None, while
preserving the existing calculations for valid serving summaries.
| usage = response.get("usage", {}) | ||
| if usage.get("completion_tokens") != args.max_tokens: | ||
| raise AssertionError( | ||
| f"expected {args.max_tokens} completion tokens, got usage={usage!r}" | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
usage can be null in the response, turning the assertion into an AttributeError.
Per pypto_serving/serving/server/server.py:135-179, usage is initialized to None and only populated when an output reports finished; an unfinished/aborted stream serializes "usage": null. response.get("usage", {}) then returns None, so Line 212 raises AttributeError: 'NoneType' object has no attribute 'get' instead of the intended diagnostic.
🛡️ Proposed fix
- usage = response.get("usage", {})
+ usage = response.get("usage") or {}
if usage.get("completion_tokens") != args.max_tokens:📝 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.
| usage = response.get("usage", {}) | |
| if usage.get("completion_tokens") != args.max_tokens: | |
| raise AssertionError( | |
| f"expected {args.max_tokens} completion tokens, got usage={usage!r}" | |
| ) | |
| usage = response.get("usage") or {} | |
| if usage.get("completion_tokens") != args.max_tokens: | |
| raise AssertionError( | |
| f"expected {args.max_tokens} completion tokens, got usage={usage!r}" | |
| ) |
🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py around
lines 211 - 215, Update the usage extraction in the response validation flow to
handle an explicit null value, using an empty mapping fallback when
response.get("usage") returns None. Preserve the existing completion-token
assertion and diagnostic for populated usage objects.
| (artifact_dir / "completion-response.json").write_text( | ||
| json.dumps(response, ensure_ascii=False, indent=2) + "\n" | ||
| ) | ||
| (artifact_dir / "completion.txt").write_text(choices[0].get("text", "")) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Artifact I/O relies on the locale default encoding. Both scripts read/write run artifacts without an explicit encoding, so behavior depends on locale.getpreferredencoding() — an ASCII/POSIX locale (common in minimal NPU containers) turns a completed profiling run into a UnicodeEncodeError/UnicodeDecodeError. server_log.open(..., encoding="utf-8") in run_profile.py already does this correctly.
.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py#L216-L219: passencoding="utf-8"to bothwrite_textcalls forcompletion-response.jsonandcompletion.txt..agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py#L85-L86: passencoding="utf-8"to bothread_textcalls forserver.logandserving-trace/trace.json.
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 216-216: use jsonify instead of json.dumps for JSON output
Context: json.dumps(response, ensure_ascii=False, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
📍 Affects 2 files
.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py#L216-L219(this comment).agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py#L85-L86
🤖 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 @.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py around
lines 216 - 219, Make artifact encoding explicit as UTF-8: update both
write_text calls for completion-response.json and completion.txt in
.agents/skills/profile-dsv4-serving-strace/scripts/run_profile.py (lines
216-219), and both read_text calls for server.log and serving-trace/trace.json
in .agents/skills/profile-dsv4-serving-strace/scripts/analyze_profile.py (lines
85-86), passing encoding="utf-8" to each call.
| Obtain exactly eight NPUs using the environment's normal resource mechanism. Run the | ||
| workflow inside that allocation. Do not assume a particular scheduler, host, device range, | ||
| model mount, Python environment, or PTOAS release. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail closed instead of assuming device IDs 0..7.
The contract says not to assume a device range, but run_profile.sh falls back to 0..7 when allocation metadata is absent. In a scheduler or container with remapped/non-contiguous assignments, this can select devices other than the eight allocated NPUs. Require --devices when no allocation variable is present, or make the 0..7 fallback explicitly local-only.
Also applies to: 49-52
🤖 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 @.agents/skills/profile-dsv4-serving-strace/SKILL.md around lines 13 - 15,
Update run_profile.sh to fail closed when allocation metadata is unavailable
instead of defaulting to device IDs 0..7. Require an explicit --devices value in
that case, while preserving scheduler-provided allocation handling and the
existing eight-NPU workflow.
| ## Validate before reporting success | ||
|
|
||
| Require all of the following: | ||
|
|
||
| 1. The runner exits with code 0. | ||
| 2. `completion-response.json` reports `completion_tokens: 20`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make expected-token validation follow the CLI override.
run_profile.sh exposes --max-tokens N, but this contract hardcodes 20 both in the success gate and the reprocessing command. A supported invocation with another limit will be rejected or reprocessed against the wrong expectation. Remove/restrict that override, or propagate the selected value into validation and --expected-tokens.
Also applies to: 110-111
🤖 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 @.agents/skills/profile-dsv4-serving-strace/SKILL.md around lines 75 - 80,
Update the validation contract and reprocessing flow around run_profile.sh so
the expected token count follows the selected --max-tokens value instead of
hardcoding 20. Either remove/restrict the override or propagate the selected
limit into the success check and reprocessing --expected-tokens argument,
preserving code-0 validation.
970aa41 to
6c233b6
Compare
2 participants
Summary
SA_PROFILEspans and Simpler host-only[STRACE]spansWhy
DeepSeek V4 profiling previously required manually assembling server launch, request, teardown, STRACE parsing, and Perfetto rendering steps. The skill packages that workflow into one deterministic 20-token run and documents the required acceptance checks.
The runner defaults to a 2 GiB ring heap, validated by a clean real-NPU run on eight even-numbered devices. The earlier 2 GiB failure on devices 8-15 was observed while other processes held substantial HBM on the odd-numbered devices, so it was not evidence that the workload inherently required a larger default.
PTO2_RING_HEAPremains overrideable for other environments and workloads.Current Simpler renamed the three public timeout variables from
PTO2_*TIMEOUT*toSIMPLER_*TIMEOUT*in runtime PR #1268. The skill now uses the effective names and documents that the former names are not compatibility aliases.Validation
task_20260801_001413_40486182827on devices0,2,4,6,8,10,12,14PTO2_RING_HEAP=2147483648Huawei is, 20 output tokens207001, orHEAP_RING_DEADLOCKclk=devrecords andsimpler.device_effective_availableis false9922afdb: ring variables, runtime logging, device STRACE, and serving worker timeout still have live readersPTO2_OP_EXECUTE_TIMEOUT_US,PTO2_STREAM_SYNC_TIMEOUT_MS, andPTO2_SCHEDULER_TIMEOUT_MShave no current runtime readerpython skill-creator/scripts/quick_validate.py .agents/skills/profile-dsv4-serving-stracebash -n .agents/skills/profile-dsv4-serving-strace/scripts/run_profile.shruff check --config ruff.toml .agents/skills/profile-dsv4-serving-stracepython tests/lint/check_headers.pypython tests/lint/check_english_only.pypre-commit run --files <changed skill files>This PR changes no serving runtime or model code.