Repository navigation
feat: distill diffusion loader and storage boundaries - #122
Conversation
Extend the existing stable-diffusion.cpp residency owner with bounded catalog identity, byte delivery, staging, and mapping cleanup guidance. Signed-off-by: GUOGUO <xwdong1998@163.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author)
Bind the shared projection to the committed loader and storage extension without changing unrelated knowledge semantics. Signed-off-by: GUOGUO <xwdong1998@163.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (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. |
Dong1017
left a comment
There was a problem hiding this comment.
LGTM for the bounded source-derived candidate scope at eb271d3844a21f9b9852a8b7a106dfa2441a142e (base 48e0d5894117be7c57f8b4ce793c0ec986f219e8). No actionable findings.
The sampled pinned loader/consumer implementations support the catalog-identity versus byte-delivery distinction, short-read propagation, callback-before-delivery ordering, loaded-state publication and cleanup guidance. The existing eight-step residency procedure is preserved; the loader case is additive and remains candidate/not_run. Source commit cdffd9475a0db4c4481a78e0e819a59b28b12dcb is the parent of the projection-only refresh; unrelated generated records change only source URL/commit.
Ready CI: all four full-check/clean installed-package jobs passed on Python 3.10/3.11; current knowledge-model-policy status is successful. These are inspected GitHub CI results, not tests rerun here.
Verification was read-only source, diff, packaging-rule and projection inspection, not an independent recomputation of every source hash or native/hardware/model/agent-efficacy execution. Parallel knowledge contributions share generated discovery records: preserve all canonical changes and regenerate from the combined source commit during integration. This is a future integration requirement, not a present inventory defect. #91 remains a living tracker; LGTM is not empirical qualification or tracker acceptance.
Signed-off-by: GUOGUO <xwdong1998@163.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author)
Signed-off-by: GUOGUO <xwdong1998@163.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author)
Preserve canonical source and validation boundaries. Signed-off-by: GUOGUO <xwdong1998@163.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author)
Preserve canonical source and validation boundaries. Signed-off-by: GUOGUO <xwdong1998@163.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author)
Dong1017
left a comment
There was a problem hiding this comment.
LGTM for the bounded integration delta at b744943dbc2a419406cdbc1f67415b0eef1cdd2a, reported base 65147a03e465dc838770474736a620e04d72337e. No actionable findings beyond the stated verification limits.
All eight stable-diffusion.cpp owner files retain reviewed Git blob identities. Current-main content is preserved outside scoped loader/storage and owner matrix additions; runtime, generator and tests match main. Combined source d959487ab1d0b2b27fad6792ef15d91f3c91674a → head changes only generated files: other 58 records change only provenance, and the SD record retains reviewed semantics. GitHub reports mergeable.
All four exact-head Ready full-check/clean-wheel jobs pass on Python 3.10/3.11; per-head knowledge-model-policy succeeds. This does not clear the separate aggregate/history failure tracked in #140.
Read-only prior-review/delta/Git-identity/projection/test-diff and CI inspection. No PR code, tests, generator or native/model experiments executed, no exhaustive source-hash recomputation and no new backend/quality/performance/agent-efficacy qualification. Previous candidate/not_run and hardware-coverage limitations remain; issue acceptance and merging are separate.
Preserve the loader and storage source extension with all current main knowledge increments. Keep main generated projections for regeneration from this combined source commit. Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author) Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Refresh discovery records from the actual combined source revision. Verify committed source hashes and actual package resource and context reads. Agent-assisted-by: Codex / gpt-6.1-sol [effort=xhigh] (author) Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Dong1017
left a comment
There was a problem hiding this comment.
Scoped LGTM for the integration delta at 0cae446eab33645eca962374cda2ecbe34bbbe3b, reported base 5b939a95d7478b1d8bff013c477972903b7de9ba, compared with previously inspected b744943dbc2a419406cdbc1f67415b0eef1cdd2a. No new actionable source/integration finding.
All eight stable-diffusion.cpp owner blobs and the original source report are unchanged. Current-main content is preserved outside the original loader/storage additions; runtime/generator/tests match main. Combined source 0e5d063 to head changes only projections; other 59 records change provenance only.
Exact-head Ready CI has four successful full-check/installed-package jobs; lint also passes. Fresh per-head policy: success, source https://github.com/InferMatrixLab/InferRSI/actions/runs/40271581200. Aggregate declarations failures/cancellations are separate and are not claimed green. Head and mergeability were refreshed before posting (mergeable=true); recheck required gates and the resulting integration tree before any merge.
Source-derived candidate/not_run boundaries remain; this does not validate a runtime/model/hardware combination.
Read-only incremental source/blob/projection/caller/test-assertion and existing discussion/CI inspection. No PR-code execution, generator rerun, model/GPU experiment, thread resolution or Issue acceptance. This records the new inspected head without reopening resolved findings or repeating the upstream audit.
Reviewer: dot / OpenAI assistant; exact runtime model identifier is not exposed.
Dong1017
left a comment
There was a problem hiding this comment.
LGTM within this PR's accepted source/software scope. Meaningful CI-blocker clearance on unchanged head 0cae446eab33645eca962374cda2ecbe34bbbe3b: Ready CI and declarations rerun are successful; exact-head knowledge-model-policy is success. This updates the prior scoped review rather than reopening resolved findings.
Integration inspected against actual main db82ec8346364667b90f4fa97fdb190150141cf0, separately from the PR API's base snapshot 5b939a95d7478b1d8bff013c477972903b7de9ba. The intervening #154/#118 changes (18 paths) do not overlap this contribution. New evaluation-planner mapped canonical directories are not changed by this PR; its source delivery is not thereby covered by a real efficacy evaluation. No new actionable integration finding or unresolved review blocker was identified.
Verification limits: incremental source/integration/prior-thread and CI inspection, no new local tests or model/hardware execution. Existing source-fidelity/packaging conclusions retain their original limits; this is not goal/campaign acceptance or issue closure. Recheck exact head, applicable checks and integration immediately before any merge, especially after another knowledge PR changes shared matrices/projections. Historical red runs remain historical, not rewritten evidence.
Preserve both canonical knowledge histories before rebuilding generated records. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Build the native projection from canonical source 17b404a. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Dong1017
left a comment
There was a problem hiding this comment.
LGTM for the repaired source/package integration at head 7688d7a5618cba2757a8435c036ce7a18e82b164, incrementally compared with previously inspected 0cae446eab33645eca962374cda2ecbe34bbbe3b.
All eight SD owner resources and original report remain unchanged. Main's Diffusers consolidation and canonical content are preserved; runtime, generator and tests retain main's versions. Source 17b404a018fe2c0d5c6457c4fc31ad641f884f5d → head changes generated projections only; the other 59 records change provenance only. Independently checked source Diffusers procedure/registry hashes against projections. The previous merge-conflict blocker is cleared (mergeable=true); no new finding.
All four Ready jobs and current declaration compliance pass. Skill Eval publisher failed; retain its separate operational follow-up.
Integration boundary: inspected actual main 0c78a7e257dfaa3573b77581a39539e8a3a5da4c; the API base remains a4a0fbd62defe2bc53ab090e1fa50bd7139663a6. Their difference is four disjoint Flare #155 paths. This is a scoped source/software conclusion, not an all-workflows-green, model/hardware efficacy, issue acceptance or merge authorization. Separate Skill Eval publication remains failed and execution skipped; do not treat those as a successful evaluation. No model/device calls, installation or workflow dispatch was performed.
Dong1017
left a comment
There was a problem hiding this comment.
Integration-status update only at unchanged inspected head 7688d7a5618cba2757a8435c036ce7a18e82b164 (PR API base snapshot a4a0fbd62defe2bc53ab090e1fa50bd7139663a6). Actual main is 38d5883b8e72e352865e214194cb22dc99253dbd; GitHub now reports mergeable=false / dirty. The prior source-scope review remains valid historical evidence, but is not current integration clearance.
Before merge, reconcile this branch with current main, preserve both accepted contributions, regenerate affected projections only where source-binding rules require it, and verify packaging plus applicable checks on the resulting head. No conflict-file attribution is made without a merge reproduction; no new content/feature requirement is introduced. This is not a new HTTP403 finding: shared publisher recovery was verified separately. No code execution, branch change, workflow dispatch or merge was performed by this review.
Preserve the reviewed SD source resources and all accepted main contributions. Omit only the redundant empty reference field required by the reader contract. Regenerate discovery outputs separately from this committed source. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Build the unchanged native projection from committed source 44bafa1. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Skill evaluationPLANNED: execution is not authorized by knowing this hash. Selected comparisons: 0 · Estimated task calls: 0. Applicable only while these revisions and inputs match the current PR. Evidence and logs · Retention: 30 days. Pipeline evidence only; offline fixed outputs do not establish skill efficacy. Plan bindings, budget and frozen examPlan SHA-256: Model/settings:
Uncovered / unsupported paths (76)
|
Dong1017
left a comment
There was a problem hiding this comment.
Scoped LGTM at 8cf699f42821303794f1f0600d48e13a46fafb24, incremental review from 7688d7a5618cba2757a8435c036ce7a18e82b164, base 51fa1d31cc4a805490ddfe8055a36c81d70c4f08.
The original substantive owner resources and distillation report are byte-identical. Sidecar change only removes empty references: [], whose reader default remains empty; no source/validation evidence was lost. Regenerated owner projections preserve semantics, unrelated records change provenance only, and current-main content is retained. Prior conflict/integration prerequisite is cleared.
flowchart LR
A[Previously reviewed source contracts] --> B[Current-main integration]
B --> C[Projection and package checks]
C --> D[Bounded software/source clearance]
Ready 40598550832 has all four full-check/installed-package Python 3.10/3.11 jobs successful; exact-head knowledge-model-policy 40606014358 succeeds. A plan with zero selected Skill Eval comparisons is not efficacy evidence.
Read-only source/delta, affected reader/default/projection tests and remote CI inspection. No local execution, model/hardware experiments or new upstream source revalidation. Original candidate/not-run, source-fidelity and host/device limits remain; this is not goal acceptance or automatic merging.
|
Merge-wave integration prerequisite at inspected head #153 changed the same generated catalog provenance records. Integrate current main while preserving both contributions, then regenerate via the existing generator rather than choosing ours/theirs wholesale. Re-run applicable Ready/package checks on the resulting head. This is integration repair, not a new content or efficacy requirement. Existing green checks predate this combined state. flowchart LR
A[Reviewed contribution] --> B[Integrate current main]
B --> C[Regenerate and verify]
C --> D[Merge]
|
Preserve the existing SD owner extension, all accepted main resources and every original matrix delta. Retain native default omission and exact unchanged inventory assertions; generate both projections separately. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Generate the unchanged native projection from committed source 7a7b34c. Preserve current main and original candidate semantics with actual source hashes. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Dong1017
left a comment
There was a problem hiding this comment.
LGTM for the integration delta at 4ea40bc, previously inspected 8cf699f, integrated main cac287f. All eight SD owner resources and the original report are byte-identical to the prior reviewed head. The merge has the expected parents, followed by a generated-only 68-path refresh; runtime/generators/tests are unchanged. Independently checked 417 source-hash references covering 352 unique Git inputs; other catalog records change provenance only. Ready CI 40635260857 passed all four full-check/installed-package jobs, and knowledge-model-policy 40635374747 is now success (re-read before posting). Verification is incremental Git/blob/projection inspection and CI, not new upstream/model/device testing. Candidate/not_run and prior source-fidelity limits remain; zero Skill Eval comparisons are not efficacy evidence. This is a PR review conclusion, not merge, #91 closure, or campaign acceptance.
Dong1017
left a comment
There was a problem hiding this comment.
LGTM 4ea40bc
Reviewer: Codex / gpt-6.1-sol [effort=ultra], actual independent reviewer. This supplement records my separately completed review; the earlier review was not submitted by this reviewer.
Verified all 352 source bindings (347 previously verified, unchanged Git blobs plus five fresh Git-byte hashes), the 67-file index and coverage view, unchanged eight SD owner resources/report, and preservation of accepted main owners/matrix rows. All four current-head Ready full-suite/clean-wheel jobs passed on Python 3.10/3.11; lint, fresh declarations and automatic plan/publication also passed. Execute is correctly skipped under the automatic workflow condition.
Candidate/not_run remains. Upstream native I/O, device/model, quality, performance and efficacy remain unvalidated. The integrator still checks the current head/base and latest applicable CI before merging.
Refs #91.
Retain the bounded stable-diffusion.cpp loader/storage extension at
b13a48219d5cdf408edcb93da6cd69b612ed03d3: catalog/source identity versus delivered bytes, ordinary non-ZIP safetensors reads, host staging/mapping ownership and immediate consumer cleanup. The original residency procedure, source dates, MIT text, readsets, construction report and candidate / empiricalnot_runlimits remain.Canonical source
7a7b34c9dc3f64726c54fac63f5322238caf047e(treed3372b1cc62e67615da569daa3042961dd34847a) merges published8cf699f42821303794f1f0600d48e13a46fafb24with actual maincac287f62375c643254af850fd294b4bdc21cb86, in that parent order. All eight current SD owner resources and the report retain published bytes; the seven original nonmetadata resources and report also retain their original bytes. The sidecar keeps the existing omission of reader-defaultreferences: []; every nondefault field and empty-reference behavior remains. Removing the exact original SD matrix additions/replacements reconstructs the complete main files. All other accepted main content, including RLT, LoopX and Ascend material, is preserved.This extends an existing registered owner. Counts remain 66 registered skills, 96 Skill documents, 114 catalogue records and 57 normalized profiles; hardware filters remain Ascend 62, CUDA 52 and CPU 48. All main test assertions, runtime and generators are unchanged. The only added library resource is
references/loader-storage-boundaries.md.Current head
4ea40bcd0d9e0eab9e25759371500ee5a5d874da(treeda0bde3d6123207811d1884ccdc0618c16252bd0) is the generated-only sole child of that source and changes exactly 68 files: 67 discovery-index files plusdocs/skill-coverage.md. One unchanged native index render passed real committed-input-set / whole-library Git guards and complete readback against the same invocation's returned values. All 352 indexed hashes bind to actual M. The other 65 main records and README change provenance only; the SD record retains the published extension with current source/registry provenance. One complete native package bundle matched all 423 committed library resources plus its record, including all eight SD owner resources and exact-revision metadata/new-loader API reads. The unchanged coverage CLI and matching--checkpassed; the other 65 main rows remain unchanged. Its input digest issha256:d3e259c8aa4da580add9b2c8c27c585b2e2f06c91201057151f91c844c507951.Three focused tests passed at this head: normalized-profile ownership/readsets, reader-default omission and complete index metadata/maturity counts. Fresh independent exact-head review and applicable current-head Linux full-render / full-check / clean installed-package Ready CI remain required before merge. No second index render, stock Windows CLI retry or WSL run was needed. This integration adds no native/backend/device/model, quality, performance or agent-efficacy result.