Repository navigation
Conversation
Extend the weight-loading owner with source-bound byte, resident-state and oracle-path checks for streamed routed experts. Preserve candidate/not_run and synthetic-fixture limits. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
Regenerate coverage and packaged skill discovery from actual source 8b29027. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
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 da3356920acac5bbf6f3ac6a4fd6de636a6d1c1c --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:
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/README.md","source_revision":"da3356920acac5bbf6f3ac6a4fd6de636a6d1c1c","status":"ready"}Provider/settings: Uncovered / unsupported paths (87)
|
Dong1017
added
the
area:knowledge
ChatGPT Codex Connector
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Scoped incremental review of #324 at fd7f8bad against the concurrently open #322 (76b680a6): the source/packaging checks described on each standalone head do not test their combined knowledge projections. One P2 conditional integration risk is noted inline: 83 changed files overlap, including shared generated cards and source-snapshot pointers. Please keep the existing knowledge integrator's sequential landing path and rederive projections from both canonical inputs after the first merge. This is a coordination recommendation, not a new merge gate or an empirical quality verdict. Reviewed the relevant source amendments, projection patches, current discussions and repo generation contract; did not run an upstream C suite, models, SkillEval or an integrated build.
| procedures use their corresponding backend; shared methods use `common`. | ||
|
|
||
| Source snapshot: [8115da15b9ef34978de500b08aa9f14ea790c35b](https://github.com/InferMatrixLab/InferRSI/tree/8115da15b9ef34978de500b08aa9f14ea790c35b/src/inferrsi/data/skill_library). | ||
| Source snapshot: [8b290275cd757ae9baa3d7e75f98e8283846853d](https://github.com/InferMatrixLab/InferRSI/tree/8b290275cd757ae9baa3d7e75f98e8283846853d/src/inferrsi/data/skill_library). |
There was a problem hiding this comment.
P2 · conditional cross-PR integration risk. #322 (Xe-Forge) also updates this generated Source snapshot pointer from the same 2baebf6e main, and its diff overlaps 83 of this PR's 87 paths, including docs/skill-coverage.md and the generated skill cards. Both heads can pass independently while a sequential merge that simply selects one projection/source revision drops or stales the other's addition. After the first PR lands, please regenerate the remaining PR's shared projections from the combined canonical sources and check that both Xe-Forge and DeepSeek references are still discoverable and the source/index --check succeeds. This is not a defect in either standalone source analysis.
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Independent source review found no additional actionable defect in the expert-cache amendment at fd7f8bad. This conclusion covers source fidelity and the observed software checks; the existing integration discussion remains applicable.
- Verified: the 15 selected upstream files match their recorded byte counts, SHA-256 and Git blob identities. The reference accurately separates buffered-byte independence from shared metadata, resident/pending ownership, actual callback selection and ordered per-
kaccumulation. - Coverage limits are preserved: the resident snapshot checks
w1; the tiny PyTorch comparison uses tolerance/argmax, while batched C output is compared bitwise with the anchored per-token C path. Missing fixtures can return exit-zero SKIP. These definitions do not establish released-model correctness or a performance gain. - Software: current Ready/declaration checks pass. The inspected 3.11 log reports 3,389 passed, 13 skipped and 284 passing subtests; its synthetic merge has exactly the head tree. The canonical-to-head delta is projection-only, and the 78 unrelated cards change only source headers.
- Next: Dong1017 retains this amendment's integration. Continue the existing sequential-integration thread; its conditional cross-PR finding is not duplicated here. Skill Eval has no executed comparison, and candidate/
not_runremains appropriate.
Evidence and review scope
- Head:
fd7f8bad0a97b1f7e398a1cf6a494fefbf853ae2; API base / observed main:2baebf6ebb815ebf07d21c41e3b3754858c0798e. - Canonical source:
8b290275cd757ae9baa3d7e75f98e8283846853d. Its head delta changes only 81 generated projection files; six canonical/manual paths comprise the remaining PR scope. - Pinned upstream:
d0402056d0da4bf7edfd2d4977de479a0db9d201. I verified the 15 selected file identities, not the author's separate 74-blob inventory claim, and traced the amendment through cache placement/reader lifecycle, aligned/buffered reads, per-token/batched consumers, CLI/oracle wiring and test/fixture definitions. The readset retains source rights/exclusions and claim boundaries. - Ready run, including the inspected 3.11 log. Tested merge
6c1ea6fb82db36a0fefffea82e006159ca9654f6has main/head parents; its tree equals head:62d25a49024f814e6ecc55abcfbe9d322ed1c049. - Per-head declarations passed. Skill Eval remains PLANNED: zero selected comparisons, execution skipped. The author's source-M index-check success establishes reported projection consistency; this maintenance session did not rerun it.
- Source claim disposition: verified within the inspected paths; unverified for native C/model execution, production-scale numerical quality, device/performance and skill benefit. The author's incomplete Windows/Python 3.13 run remains incomplete; hosted 3.10/3.11 evidence is separate.
- Independent maintenance review; I did not implement the amendment. This run used static reads, hashing and existing CI logs only. No upstream suite, model/device work, local suite, combined #322/#324 build or paid trial was executed. Concurrent #322 head is
76b680a6a2be842e7999ffaf4a1a31c76c3dfac2; standalone checks do not qualify that combination. This COMMENT does not grant formal approval, merge permission or governing-Issue acceptance.
Retain the bounded expert-cache source references and all current-main canonical, matrix, applicability and release additions before regenerating projections. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Rebuild the native catalog and coverage from the actual combined canonical source revision without importing other pending source targets. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
![agent-ci-infra-bot[bot]](https://avatars.githubusercontent.com/u/343315635?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed commit: 1003fa8
Coverage: Snapshot: 2647 files, 42500005 bytes; skipped 0 entries; Initial model context: 28 files; other accepted files available through tools; Git workspace contains a fixed snapshot, not full upstream history
| "refusal_point": "Withholds a load-path win until the verification checklist holds and keeps the distributed-delivery checks as verification targets rather than results; does not force prefetch without a safe dependency window and disables it when neither a smaller size nor a moved position removes a regression; records unsupported save behaviors as unknown; rejects mismatched execution ABI or prepared-contract versions instead of editing their tags; and implies no Ascend support from the llama.cpp or fastsafetensors mechanisms", | ||
| "hardware": "NPU for the device-side prefetch API (torch_npu.npu_prefetch, checked against the target CANN version) and the Diffusers offload playbook; the prefetch and fastsafetensors methodology transfers while the fastsafetensors and llama.cpp APIs do not, and cross-rank completion follows each collective backend's own wait contract" | ||
| "refusal_point": "Withholds a load-path win until the verification checklist holds and keeps the distributed-delivery checks as verification targets rather than results; does not force prefetch without a safe dependency window and disables it when neither a smaller size nor a moved position removes a regression; records unsupported save behaviors as unknown; rejects mismatched execution ABI or prepared-contract versions instead of editing their tags; and implies no Ascend support from the llama.cpp or fastsafetensors mechanisms; withholds expert-cache correctness/overlap conclusions when byte coverage, actual I/O/pool dispatch or model-oracle callback reachability is missing", | ||
| "hardware": "NPU for the device-side prefetch API (torch_npu.npu_prefetch, checked against the target CANN version) and the Diffusers offload playbook; the prefetch and fastsafetensors methodology transfers while the fastsafetensors and llama.cpp APIs do not, and cross-rank completion follows each collective backend's own wait contract; the DeepSeek source slice describes POSIX storage, C/OpenMP and pthread ownership; target validation must identify its actual direct/buffered and reader-pool paths" |
There was a problem hiding this comment.
F1 — medium, suggestion — applicability.hardware now carries the upstream slice's platform facts
- Where:
src/inferrsi/data/skill_library/methods/weight-loading-optimization/metadata.json:125 - Problem:
docs/knowledge-base.md:99defineshardwareas "A device dependency the body establishes" and forbids it being "inferred from a source's hardware". The added clause — "the DeepSeek source slice describes POSIX storage, C/OpenMP and pthread ownership; target validation must identify its actual direct/buffered and reader-pool paths" — describes the source's language/OS and a validation instruction, not a device dependency of the procedure. It is copied verbatim into the generated card atsrc/inferrsi/data/repo_packs/inferrsi/knowledge/common/skills/weight-loading-optimization.json:190and:236. - Conditions: any read of the sidecar or the generated catalog card; no runtime condition needed.
- Impact: consumers use this field to judge device applicability; a reader of the catalog can no longer tell NPU/CPU device scope (
torch_npu.npu_prefetch, Diffusers offload) apart from the upstream source's build environment, and the field silently drifts from the documented contract the other sidecars follow. - Fix: keep
hardwareto the device dependencies and move the platform note intoapplicability.prerequisitesorscope(e.g. "the audited source is POSIX/C/OpenMP/pthread; identify the target's actual direct/buffered and reader-pool path before applying"), then regenerate the projection.
|
|
||
| | Source | Inspected scope | Canonical extraction and boundary | | ||
| | --- | --- | --- | | ||
| | [ronak-create/deepseek-v4-in-c](https://github.com/ronak-create/deepseek-v4-in-c/tree/d0402056d0da4bf7edfd2d4977de479a0db9d201) @ `d0402056d0da4bf7edfd2d4977de479a0db9d201` (Apache-2.0 root and selected-file declarations; third-party/model rights excluded) | Bounded source inspection, 2026-11-03: byte/fixture geometry, begin/end slot lifetime, actual model/oracle callback wiring and tiny-reference limits; [selected hashes, regions and rights](../src/inferrsi/data/skill_library/methods/weight-loading-optimization/references/deepseek-expert-cache-source.json) | [weight-loading-optimization reference](../src/inferrsi/data/skill_library/methods/weight-loading-optimization/references/deepseek-expert-cache-oracles.md): independent buffered bytes, usable residents during misses, ordered accumulation and oracle reachability. Source-derived candidate/not_run; historical corruption reports are not reproduced tests, and tiny fixtures do not establish released-model quality or performance. | |
There was a problem hiding this comment.
F2 — medium, suggestion — the new Reference Matrix row is attributed to no skill, so the regenerated coverage view omits it
- Where:
docs/reference-matrix.md:554 - Problem:
tools/build_skill_coverage.py(references()) attributes a matrix row to a skill only when the row contains a backticked registered id or a link that resolves inside<skill-id>/; every link in this row targets.../weight-loading-optimization/references/…, so the resolved parent name isreferencesand the row never maps toweight-loading-optimization. I confirmed this by running the generator: thermentries forweight-loading-optimizationare still only the six older "Systems sources" rows, and the regenerateddocs/skill-coverage.md:145shows no DeepSeek entry. The neighbouring Flare precedent (docs/searchspace-matrix.md:551) links the ownerSKILL.mdand is attributed. - Conditions: every
python tools/build_skill_coverage.pyrun, including the one in this PR. - Impact: the repository declares the coverage view to be generated from the two matrices; the new source/extraction mapping is therefore invisible in the maintained projection, so the delivered knowledge looks smaller than it is and future audits of "which source row covers this owner" cannot find it there.
- Fix: name the owner in the row (e.g. add
[weight-loading-optimization](../src/inferrsi/data/skill_library/methods/weight-loading-optimization/SKILL.md)or backtick the id) and regeneratedocs/skill-coverage.md; the same link belongs in thedocs/searchspace-matrix.md:622row if that mapping should also surface as owner guidance.
|
This PR extends the Reviewed commit: 1003fa8 |
agent-ci-infra-bot
Bot
added
bot-reviewed
kind:docs
Keep source platform prerequisites separate from device dependencies. Attribute the source matrix rows to the existing owner for native coverage. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Generate native coverage and catalog from real corrected source da33569. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
The two applicability/coverage comments have corresponding fixes at f486031f; this is a bounded update to the earlier source review.
- The POSIX/C/OpenMP/pthread note now lives in applicability
scope, whilehardwareretains the device-dependency wording required by the documented contract. Both applicability copies in the generated owner card match this separation. - Both matrix rows now link the registered
weight-loading-optimizationentry. The generated coverage row includes the DeepSeek reference and its supplementary RAM/SSD guidance; this is attribution coverage, not measured resource benefit. - The two audited DeepSeek reference resources are unchanged from the earlier reviewed head. The corrective source-to-head delta changes only 81 projections; all 78 unrelated cards change only their source URL/commit headers.
- Dong1017 retains integration and the existing sequential-landing discussion. Current software checks pass, while Skill Eval remains PLANNED with zero comparisons; neither that report nor this repair confirmation supplies model efficacy, whole-task acceptance or a new formal approval.
Evidence and limits
- Head:
f486031ff9865c8f280a501272dc0e632d86f983; API base / assessed main:663f3b327998526a13aa31e72ad729025c18983c; corrective canonical source:da3356920acac5bbf6f3ac6a4fd6de636a6d1c1c. - Original comments: applicability field, matrix-owner attribution. I have not resolved those threads.
- Inspected current canonical metadata, both matrix rows, the owner card and coverage output against the field contract. Checked the complete canonical-to-head delta: no canonical-library change, 81 projection paths, and no unrelated-card content drift after the two source-pin substitutions.
- The prior 15-file primary-source review remains revision-bound to
fd7f8bad0a97b1f7e398a1cf6a494fefbf853ae2; the unchanged reference bytes carry that evidence forward, rather than claiming 15 fresh upstream reads. - Current hosted software job, current declaration policy, Skill Eval plan. I did not run local/native tests, the index-check command, an integrated multi-PR build, models or hardware workloads.
|
Source-only knowledge amendment that adds a DeepSeek streamed expert-cache reference/readset to Reviewed commit: f486031 |
Refs #91. Refs #177.
Problem and scope
Serial and concurrent expert-cache loads can agree while sharing the same byte-placement error. This source-only amendment extends the existing
weight-loading-optimizationowner with an operational reference and an ordinary source-readset JSON, an entry pointer, incremental metadata and the two matrix mappings.The guidance separates buffered byte-oracle independence from shared metadata, resident bytes from unpublished asynchronous misses, ordered caller accumulation from read completion order, and actual production callback coverage from a test wired only to synchronous access. It retains direct/buffered dispatch, reader-pool fallback, partial-I/O failure, cleanup and tiny-fixture limits.
Fixed source and provenance
d0402056d0da4bf7edfd2d4977de479a0db9d201.8b290275cd757ae9baa3d7e75f98e8283846853dis retained through integration M0a14fc704dedc5146fefb31073de74c079d3690eover actual main663f3b327998526a13aa31e72ad729025c18983c. Corrective canonical Mda3356920acac5bbf6f3ac6a4fd6de636a6d1c1cfollows prior H1003fa822a4377162112d94d5d064606301780a7; native G/H isf486031ff9865c8f280a501272dc0e632d86f983. It moves the unchanged source-platform note fromhardwaretoscope, restores the main hardware wording, and makes both matrix rows attributable to the existing owner. The original parser adds RAM/SSD supplementary guidance without changing numbered stages. The schema and generator are unchanged; both source resource files retain their original bytes.Validation and limits
Both original native write modes passed from actual corrective M. The actual parser was checked before and after the three-file source change: one DeepSeek Reference Matrix row and one supplementary Searchspace row now belong to the existing owner; numbered resource stages and unrelated mappings remain unchanged, with no dead links.
Both original final-H checks passed with 79 current metadata records and matching coverage. Native owner/catalog reads verified all 8 source-file manifest entries; checkout returned 7 resources and the source-package adapter exposed 757 dependency resources. Whole-resource Collect selected 6 resources (74,108 bytes), including both unchanged source references, with no omissions and revision-bound byte/hash checks. Actual-H checks also confirmed the restored hardware wording, preserved scope note and visible DeepSeek owner mappings. Source-package reads use this checkout and are distinct from installed-wheel evidence.
Ready 41627213110 passed all six jobs. Python 3.10 and 3.11 full checks each reported 3,544 passed / 13 skipped / 1 warning / 284 subtests, plus Ruff. Each fresh isolated noneditable wheel loaded 127 source-grounded records from site-packages outside the checkout and verified its declared resources. The actual tested merge
66f9ea9fb735adc64c856542e64d83699584a2a7, with main663f3b32and this H as parents, has exactly this H tree.Private independent gpt-6.1-sol / ultra review covered the complete corrected H, all source/native preservation, the two findings' actual outputs, readers and current software/wheel evidence: LGTM, with no current-H findings. The reviewer is review-only. Current-H Codex review completed with no findings or unresolved flags. Previous F1/F2 threads remain unmodified and outdated; their content is corrected and independently verified. The conditional future-integration thread remains retained. Current PR checks report the final head-bound declaration result. Earlier CI and LGTM at
1003fa82remain historical.The original source test definitions and tiny-fixture limitations remain unchanged. The earlier supplemental Windows/Python 3.13 two-file test timed out and is still incomplete. SkillEval supplied no executed comparison or efficacy result.
Knowledge remains candidate/not_run and efficacy remains not_evaluated. No upstream C/cache/model suite, device/model execution, benchmark, quality/service experiment or paid model trial was run.
Knowledge authorship declaration