Repository navigation
Conversation
Trace Faster CosyVoice PCM credit, dependent chunk batching and measured cost boundaries through the existing benchmark method. Retain source-only candidate status, UNKNOWN repository rights and hardware applicability. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Build the complete native catalog and coverage from source revision 8daa933. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Skill evaluationPLANNED: not executed; there are no task outcomes yet.
Author verification steps (nonpaid; not yet run)
python tools/build_skill_index.py --source-revision b2d62f8192903ab31f8e8828f4e630a1bec776b7 --checkThe declared source pin matches this head's canonical library bytes. The command has not run: retain its exit code and full drift/success output. It checks the generated projection, not claim truth or skill efficacy.
Evidence and logs · Retention: 30 days. Plan bindings, budget, frozen exam and advisory detailsAdvisory: obtain missing evidenceRetained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
Model/settings:
Selection/task metadata (not a relevance verdict): {"adapter":"audit","basis":"trusted_path_mapping","matched_paths":["src/inferrsi/data/skill_library/methods/inference-benchmark-design/SKILL.md","src/inferrsi/data/skill_library/methods/inference-benchmark-design/metadata.json","src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/dependency-aware-tts-playout.md","src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/faster-cosyvoice-source.json"],"tasks":[{"evidence_scope":"http_fixed_workload","id":"audit-threshold","request":"Audit the fixed input report using its contract. Compute the median of per-trial request means, including every request in each trial, and the percentage reduction relative to the baseline aggregate.","split":"holdout"}]}Automatic execution / cumulative PR budget: {"reason":"Trusted configuration or pricing missing, invalid or expired","remaining_usd":"unknown","reserved_or_unresolved_usd":"unknown","spent_or_legacy_usd":"unknown","state":"blocked"}Projection check selection (not an executed result): {"declared_in":"src/inferrsi/data/repo_packs/inferrsi/knowledge/_provenance.json","source_revision":"b2d62f8192903ab31f8e8828f4e630a1bec776b7","status":"ready"}Provider/settings: Uncovered / unsupported paths (9)
|
|
This PR adds a source-bound Faster CosyVoice reference and per-file readset under the existing Reviewed commit: 8811590 |
|
This PR adds a pinned Faster CosyVoice source record and an original "dependency-aware TTS playout" reference to the existing Reviewed commit: 8811590 |
Dong1017
left a comment
There was a problem hiding this comment.
Source-only LGTM at 88115905. I found no blocking defect in the new Faster CosyVoice interpretation or its current knowledge projection.
- The source supports the distinctions between PCM receipt/playback and server credit, dependent same-session chunks, accumulated vocoding work, and token-derived versus delivered-audio denominators.
- Overall rights remain UNKNOWN; file-header exceptions, inherited attribution and author-reported quality/performance limits are retained. This adds methodology guidance to the existing owner.
- @Dong1017 retains integration. If another pending knowledge amendment lands first, preserve the combined canonical inputs and regenerate the affected projections. This standalone review does not test their combination or accept the broader tracking goal.
Evidence and limits
Reviewed head 881159057101c639504bd16d889c81cb5d514ee1; API base and actual main 663f3b327998526a13aa31e72ad729025c18983c; canonical source M 8daa93343cb7a33069b6c6b53a82eb4ec14941a0.
Independently fetched all 33 recorded upstream files at yuekaizhang/faster-cosyvoice@f1049c59196c2c1a890079abc56056b43f27cd1d, matched their UTF-8 byte lengths/Git blob identities to the readset and complete 117-entry tree, and checked the relevant technical and rights ranges. Checked the 87 changed paths, canonical/matrix additions and generated delta: M→H changes only 81 projection paths; 78 unrelated cards only replace the source pin. All 12 declared owner-resource SHA-256 values match actual H bytes; both applicability copies match the sidecar.
Hosted Python 3.11 checks: Ruff success; 3,544 passed / 13 skipped / 284 subtests. Actual checkout 866297f8c69267afd79fb38848a8fb6297df4387 merges this H into the assessed main and has zero file differences from H. Both installed-package checks succeeded. The per-head policy reports compliant for this exact H/main. Public current-H generator/read checks are attributed separately; I did not rerun them.
SkillEval remains PLANNED with zero comparisons/calls and an unsupported-snapshot limit; this is not efficacy evidence. No upstream tests, model/device/audio/service trial was run. This maintenance chat did not implement the change. The conclusion is a COMMENTED review, not a formal APPROVE, merge authorization or goal acceptance.
|
Review at head
Not verified by me: the upstream source pins/byte hashes in |
|
Re-review at head No new commits since the prior short-SHA note. Isolation content (Faster CosyVoice reference) still looks coherent, but blockers remain:
Not verified by me: upstream source pins/byte hashes in Reviewed commit: |
Merge main after the earlier source PRs and the shared source snapshot landed. Where this PR and main extend the same files, keep main's additions first, then the Faster CosyVoice additions. Generated projections keep main's bytes until they are regenerated from this commit. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Run main's writers on the committed combined source. Only the edited owner's record, the shared source snapshot and the coverage view change. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base 14fa1f578655bbdcd101b8c4235299fc3d8360d3 and head a0978f59a24115e60e894a4bfdd5f57ea3f37ddb. Full diff (previousHead unset).
Verified in this revision:
references/dependency-aware-tts-playout.mdis source-derived candidate guidance foryuekaizhang/faster-cosyvoice@f1049c59196c2c1a890079abc56056b43f27cd1d. It keeps server route credit, client zero-buffer playback, same-session Mel dependence, cumulative vocoding, and token-derived versus delivered-audio denominators distinct. Cited line ranges fall inside the inspected ranges inreferences/faster-cosyvoice-source.json. Status stays candidate / empirical not_run / efficacy not_evaluated.- Overall source rights stay UNKNOWN. The source record keeps selected Apache-2.0 file headers (Alibaba
flow.pyand HiFTgenerator.py; NVIDIAflashinfer_dit.pyandflashinfer_flow.py) and unresolved HiFT / Nari inheritance. The new reference is original analysis and immutable links. - The pin, 33-path readset, license string, and not_run caveat match across
inference-benchmark-design/metadata.json,knowledge/common/skills/inference-benchmark-design.json(sources, limitations, and provenance readsets),docs/reference-matrix.md, anddocs/searchspace-matrix.md. Searchspace adds methodology context only; the skill-coverage hardware N/A reason is unchanged. Coverage readset count moves from 11 to 13 with the two new provenance readsets. The skill-index registry hash is unchanged.
Not verified here: the 33 upstream blobs and tree a5950f9eaa500141d08c29e29dc24e8d1f313f00 were not re-fetched; python tools/build_skill_coverage.py --check was not run; projection source_files sha256 values and source_revision b2d62f8192903ab31f8e8828f4e630a1bec776b7 were not recomputed; tests and CI were not executed.
No blocking defect on this head. The source-only comment at 88115905 is an earlier author check and is not the evidence for this review.
amy-why-3459
left a comment
There was a problem hiding this comment.
LGTM
Incremental review of 881159057101c639504bd16d889c81cb5d514ee1..a0978f59a24115e60e894a4bfdd5f57ea3f37ddb (merge of main 14fa1f578655bbdcd101b8c4235299fc3d8360d3 in b2d62f81, then projection regeneration in a0978f59).
Verified:
- The earlier blockers are resolved: the PR is now mergeable (
clean) against current main, and #322/#324/#325 have landed. The hand-writteninference-benchmark-design/SKILL.md(+6) andmetadata.json(+48) are purely additive on top of main; the only-lines are comma moves, so the NInfer and other main-side entries are preserved. - The diff against main shrank from the old 80+ regenerated files to 9 files; only
inference-benchmark-design.json,_provenance.jsonanddocs/skill-coverage.mdchange in the generated projection. - The
source_fileshashes in the regeneratedinference-benchmark-design.jsonmatch the head bytes ofSKILL.md,metadata.json,dependency-aware-tts-playout.mdandfaster-cosyvoice-source.json. - All checks are green on this head (declarations on its latest run, pr-references, full-check 3.10/3.11, installed-package 3.10/3.11).
Still not independently verified by me: the upstream byte hashes in faster-cosyvoice-source.json. Overall upstream rights stay UNKNOWN, as the PR itself states. #342 still edits the same inference-benchmark-design files and will need to rebase after this one.
|
@zengchuang-hw PTAL |
zengchuang-hw
left a comment
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base 14fa1f578655bbdcd101b8c4235299fc3d8360d3 and head a0978f59a24115e60e894a4bfdd5f57ea3f37ddb. Acting as the independent reviewer under zengchuang-hw's delegated review authority, responding to the review request. I did not implement this change.
This is my first substantive review of this PR: I examined the full nine-path diff against current main and reconciled the integration range 881159057101c639504bd16d889c81cb5d514ee1..a0978f59a24115e60e894a4bfdd5f57ea3f37ddb. Amy's previous review is the historical integration baseline, not my own earlier review.
Verified:
- The hand-written changes preserve main's existing guidance and add the Faster CosyVoice method/reference without creating a runtime, hardware coverage or efficacy claim. The two new reference resources are byte-identical to 8811590. The combined-source commit b2d62f8 includes main 14fa1f5; b2d62f8→head changes only the owner projection, shared provenance and coverage view.
- Independently retrieved all 33 pinned upstream files. Their connector blob identities and complete 117-entry tree sizes/identities match the readset. Traced the actual client PCM alignment/zero-buffer clock, server route credit and timer boundary, chunk planner, cross-session batch exclusion, cumulative Mel vocoding and offline token-duration accounting. These support the reference's distinctions; reported benchmark/quality results remain author reports.
- Recomputed SHA-256 from the four changed owner-resource contents and matched the projection. Source metadata agrees between sidecar and generated record. The source has no repository-level license/notice path in the pinned tree; UNKNOWN rights and file-level exceptions remain explicit.
- Read AGENTS, review/contribution/knowledge contracts and governing #91/#177 scope. Earlier integration conflicts are resolved for the assessed main. This does not validate a future combination with #342.
Checks and evidence:
- Independently performed the source/tree identity, four SHA-256 and commit/tree comparisons above.
- Inspected current-head hosted Ready checks: Python 3.10/3.11 full-check and installed-package checks plus pr-references succeeded. Read the Python 3.11 log: Ruff passed; 3,582 tests passed, 14 skipped and 286 subtests passed. Installed-wheel log reports 128 source-grounded records. Tested merge 1593af3 has exactly the same tree as this head (8899009dd667247e03a3c5f80366fd128a34c4fd).
- Latest declaration run reports compliant for this exact head; an earlier declaration failure is superseded by that corrected evaluation. SkillEval execute is skipped and its plan remains unexecuted, not efficacy evidence.
- I did not rerun the native generators or test suite locally, execute upstream tests, or run models/devices/audio/service benchmarks. Hosted evidence is attributed above, not presented as my local execution.
Blocking findings before approval: none for this bounded source-guidance delivery.
Nonblocking integration note: #342 still needs its own current-main reconciliation and evidence refresh. Approval is not whole-issue acceptance, source licensing clearance, measured efficacy or merge authorization.
Streaming TTS measurements can conflate the server's playback estimate with the client's arrival and playback clock, and can omit accumulated synthesis work. This change adds a source-bound interpretation to the existing
inference-benchmark-designowner for PCM sample boundaries, dependent chunk batching, accumulated vocoding, and timing denominators.The source is yuekaizhang/faster-cosyvoice at f1049c59. The readset records inspected files and their individual rights. Overall repository rights remain UNKNOWN; selected Apache-2.0 headers do not extend to other files. The reference contains original analysis and immutable links.
Current-main integration (head
a0978f59a24115e60e894a4bfdd5f57ea3f37ddb)This follows the agreed order after #332, #322, #360, #324, #325 and #359 landed (main
14fa1f578655bbdcd101b8c4235299fc3d8360d3), and addresses the cross-PR integration blockers in the earlier review. The branch fast-forwards from the previous head881159057101c639504bd16d889c81cb5d514ee1; no history was rewritten.b2d62f8192903ab31f8e8828f4e630a1bec776b714fa1f57a0978f59a24115e60e894a4bfdd5f57ea3f37ddbb2d62f81Xe-Forge (#322) and NInfer (#325) already extend
inference-benchmark-design, so four hand-written files needed a real merge. Every overlap is additive; main's additions come first, then this PR's:metadata.json: merged by JSON field. Main's 3 added sources, 2 limitations and 4 readsets stay; this PR appends the Faster CosyVoice source, 1 limitation and 2 readsets. Any concurrent edit that is not a pure append would have stopped the merge.SKILL.md: the 6-line dependency-aware TTS playout paragraph keeps its original place, before the treatment-delivery paragraph.docs/reference-matrix.md: the Faster CosyVoice row (1 line) is the last row of the source table, after main's rows.docs/searchspace-matrix.md: the 13-line Faster CosyVoice section follows main's sections at the end of the file.The two new reference files are byte-identical to the previous head. With the shared source snapshot from #359, regeneration changed 3 generated files: the
inference-benchmark-designrecord,_provenance.json(now pinned tob2d62f81) anddocs/skill-coverage.md. Relative to main, the head changes 9 paths: the 6 canonical paths and these 3.Commands run at this head, all exit 0:
git merge-tree --write-tree 14fa1f57 a0978f59is conflict-free and produces exactly this head's tree8899009dd667247e03a3c5f80366fd128a34c4fd.A local focused test run at this head did not complete: the host stopped it for low memory, so it gives no result. Hosted Python 3.10/3.11 full and installed-package checks are the test evidence for this head.
Earlier head evidence (
881159057101c639504bd16d889c81cb5d514ee1)The following results apply to the previous head over main
663f3b32and are not substituted for current-head checks.Canonical source M:
8daa93343cb7a33069b6c6b53a82eb4ec14941a0. Native generated G and head:881159057101c639504bd16d889c81cb5d514ee1.The native coverage and index write/check modes passed from the actual M; the index contains 79 records. Final-head source readers and whole-resource Collect read both new resources completely, with 12 selected resources and no omissions. Seven existing local test modules passed with 104 passed and 5 skipped. A Windows checkout initially materialized the new JSON with CRLF, causing a hash mismatch; restoring the committed LF blob resolved the mismatch without changing the Git tree or relaxing validation.
Ready CI passed on the tested merge of then-current main and that head; its tree equals that head's tree. Python 3.10 and 3.11 each reported 3,544 passed, 13 skipped, 1 warning and 284 subtests; both isolated installed-wheel checks read 127 source-grounded records. The head-bound model-policy check passed. Independent Sol/ultra static review covered that head and found no blocking issue; it did not run the source or evaluate efficacy.
Limits
Source observations and author-reported measurements remain distinct. This candidate is
not_runand efficacy isnot_evaluated; upstream execution, device, audio quality, service and performance have not been evaluated. The snapshot pinb2d62f81is a branch commit; after a squash merge it is not an ancestor of main, and the next knowledge PR re-pins when it regenerates. The pending Embodied increment (#342) is not imported; it will be integrated on top of this PR after it lands.Refs #91. Refs #177.
Knowledge authorship declaration