Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
Retain authoring and Pumas documentation contracts, preserve the exact inference producer pin, and record fresh source validation separately from outstanding trained-model/native acceptance.
|
Disposition: retain this PR as draft; do not close its unique Pumas chat/embedding, reference-index and lifecycle work. The dependency-security repair is already retained on main via the authoring integration. Integration commit b4af7e9 normally merges main 6bef187 into be03aa1, preserving both histories and the ad31e391 producer pin. Six documentation conflicts retain both contracts; source merges mechanically. Independent GPT-6.1 Sol High review found no new confirmed P1/P2 and separately checked consumed discovery/DTO/typed operation/SSE/metadata/cursor contracts against the exact producer. The updated readiness report preserves the original evidence. Fresh local composed-tree validation passes: 129 core + 638 server tests and all example targets; strict all-target/all-feature core/server Clippy; Rust formatting and decision traceability; five ONNX guard unit tests and seven authoring fixture regressions; pinned Node npm ci/lint/format/typecheck, all 115 files / 646 frontend tests and static build. Dependency checks pass with three low cookie-related rows; cargo-audit/deny/machete remain explicit missing-tool skips. GTK/glib/WebKit development packages are absent locally. Exact-head CI run 110 is queued; hosted results will be recorded separately. Blocking merge: actual trained chat + embedding and native lifecycle acceptance is still unexecuted. An operator with accessible local model packages must run the acceptance procedure, recording real generation/structured output, indexed reference contribution in the consumed prompt, reopen/reindex, same-model reload invalidation, visible failures, and owned/borrowed cleanup; then the owner must disposition that runtime gate. Synthetic fixtures, generic CI and earlier native authoring captures do not establish it. No model/credential acquisition or denied-access retry was attempted. The documented lower-priority borrowed-bootstrap timeout observer issue remains deferred. |
|
Additional acceptance findings from independent review against current Coding-Standards 0fc92a24: mechanical source integration b4af7e9 is approved, but this does not establish complete product acceptance or standards compliance. Lifecycle ownership remains incomplete in pumas_runtime.rs: the admitted startup task at line 45 has no retained completion/panic observer after caller cancellation and can discard a start error; the stdout drain at line 118 discards its handle and I/O result; Drop cleanup at line 207 detaches its fallback completion. The known borrowed-bootstrap timeout at lines 121–128 also drops its child observer. Rust async ownership and concurrency ownership require an actual terminal/error/shutdown owner. The inference lifecycle owner must repair these paths or record an explicit owned, justified exception before final acceptance. No lifecycle feature lane was opened during cleanup. The synchronous shutdown wait in src-tauri/src/lib.rs:86 deliberately retains the drain receipt before exit. Its callback/exit execution contract and accepted design rationale still need to be established. This is a design/acceptance concern; the reviewer did not demonstrate deadlock or an async-worker-thread violation, and block_on is not categorically forbidden in all synchronous callbacks. These findings reinforce retaining the unique PR as draft. They do not invalidate the mechanical integration or authorize discarding its work. Required real-model/native evidence remains separately blocked, as recorded above. |
|
Fresh exact-head CI run 110 is now fully successful for b4af7e9: Linux/Windows full-workspace Clippy and tests, frontend audit/lint/format/typecheck/tests/static build, ONNX no-download safeguards, authoring regressions and decision traceability all pass. The admitted GitHub merge-test tree matched the reviewed head tree exactly. Source integration is validated. Disposition remains retain draft / blocked for product acceptance, for the real-model/native operator procedure and the lifecycle ownership/design findings recorded above. No generic CI result is being used as trained-model or native interaction acceptance. README-only PR33 has been separately reviewed and normally merged as 728a7d6; no PR29 product work was merged or discarded. |
Retain child/output results through explicit shutdown and reap the spawned borrowed helper on timeout without touching its service. Keep cancelled-construction and last-Drop observation gated on the documented composition-owner decision.
|
Published partial lifecycle repair 338890f3bc4c61bf5908b6616a32121a41ecf8b3, a normal child of b4af7e9 with no history rewrite. Independent GPT-6.1 Sol High review approved the exact five-file tree 8550c4f8 narrowly. The existing runtime owner now retains child + stdout completion, reports stdout I/O errors/panics as sticky shutdown failures, and retains the handle across a cancelled shutdown waiter. A timed-out borrowed bootstrap has only our spawned helper killed/reaped; no service shutdown/unload RPC is sent. The new regression first failed on b4af7e9 with Local checks pass: six targeted lifecycle tests; 129 core + 642 server tests and all example targets; strict all-target/all-feature core/server Clippy; formatting/traceability; configured pre-commit lint/format/typecheck and commit-message checks. Canonical Remaining material decision, not silently implemented: an owner must be retained by the application composition root before subprocess admission and explicitly drained after cancelled initialization/last-Drop cleanup requests. Current cancellable The synchronous Tauri shutdown contract was source-reviewed at locked 2.11.2; no demonstrated deadlock or native execution acceptance is claimed. Required native exit qualification and actual trained chat/embedding acceptance remain open; no models/credentials or denied-access retries were acquired. Keep draft. |
Reviewed composition-owner proposal (no API implementation)Source baseline: CI112 / run 39710649846, source head338890f3: attempt1 is terminal failure. Linux Rust1016 passed, Windows Rust1010 passed; two native-display tests ignored on each. Traceability passed. Frontend Full reviewed proposalPR29 retained composition owner: implementation proposalStatus: proposed, awaiting the construction/API decision; no implementation is Decision requestedPreserve the existing cancelled-startup and eventual-cleanup guarantees by The proposed API changes below require an explicit decision before source Small ownership boundaryAdd a Pumas-specific The owner holds one admitted Pumas session for the one AppState at each current Each owner entry retains (a) the control-task JoinHandle and its terminal result, The stdout reader stores validated bootstrap ownership, service description and Cancellation, panic and terminal custodyThe control task remains admitted through startup, facade lifetime and cleanup. Last facade Drop only signals the already admitted task, synchronously. It Use one retained session state machine: admitted -> starting -> attached or Expose If drain itself is cancelled, another call on the same retained owner resumes Exact caller migration
Desktop (
Tauri's Builder::run/App::run do not provide a post-return cleanup phase. Backend smoke ( Examples: migrate all five roots in Tests: migrate 44 construction calls in 24 server files and the three calls at Externally callable signature decisions
Add only the owner/handle/receipt/error types and their constructor, handle, An additive migration can add Bounded implementation and evidence gates
This proposal adds no feature lane, generic task framework or public API |
Local chat and reference embeddings now use independently selected Pumas models/runtime profiles through Pumas's serving lifecycle and typed model-operation v1, pinned to ad31e391dcd94c204b3fcd748cc98d27b3281e65 (0.8.0-rc.1). Explicit Load/Unload controls and capability/error checks preserve existing direct llama.cpp and OpenRouter providers.
Reference indexing publishes complete documents only after successful embeddings, binds exact source/session/provider/model/profile/provenance/load revision and dimensions, reindexes saved references on project reopen or embedding-selection changes, and exposes ready/stale/failed status plus explicit Reindex. Generation reports consumed reference sources or retrieval errors; Last Generation Prompt shows the retrieved text included in the actual generation request. Optional Linux selected-root Pumas bootstrap drains only an owned service, observes its child-exit receipt before desktop exit, and survives cancellation during admitted startup. Borrowed services remain running. Explicit startup failures preserve Pumas selection and display the actual error.
Follow-up review repairs: unload decodes the pinned producer's typed UnserveModelResponse, rejects success:true/error:Some/unloaded:false, preserves error-free unloaded:false as already unserved, and rejects missing receipts. Retrieval checks the captured scope while holding the index guard before any compatibility/status mutation. Exact regressions first reproduced the old failures, then proved a fresh newer-revision/dimension index remains ready and usable after late queries across Reindex and same-path reopen.
Validation on repair head b09f9c2: 123 core + 611 server tests and all-target/all-feature core/server clippy passed locally; frontend lint/format/typecheck and 606 tests passed; formatting and decision traceability passed. Exact-head CI run 96 is fully green: Linux and Windows workspace clippy/tests, frontend lint/format/typecheck/tests, ONNX no-download guard, authoring regressions and decision traceability. Linux passed 977 Rust tests; Windows passed 975. Both platforms retain two existing ignored native-display renderer tests.
Controlled producer/process fixtures cover load/embed/unload, failures, revision/model changes, source upload/reopen/retrieval-to-chat prompt, typed streaming/cancellation, and owned versus borrowed cleanup. Full local desktop build was blocked by missing GTK/WebKit development libraries; the pre-push desktop hook failed at glib-2.0 and was excluded for push after the listed checks passed. The repair head subsequently passed full native workspace checks in CI.
No real-model inference was executed because no embedding/chat weights are available in this environment; fixture outputs are test data only, never production fallbacks. Native-display acceptance remains unexecuted. Pumas typed v1 supplies no immutable weight revision in each response, so identity is session/provenance-bound and out-of-band weight mutation without metadata/reload remains a producer limitation.
Operator setup and real-model acceptance: docs/pumas-local-inference.md. Pumas producer unchanged. Keep draft; do not merge without coordinating with the parent task.
Integration and review update — 2026-10-14
Current head:
1148fb0e44efc25420e1b71494eef6d7cae99840. This merge commit preserves original PR headb09f9c28and main59793c19as parents, including PR30's native UI fixes. Pumas deliberately remainsad31e391(0.8.0-rc.1): main's older setup pin lacks the discovery and typed-operation APIs this consumer uses.Independent GPT-6.1 Sol High review found a genuine mismatch: the pinned producer omits optional
loaded_at, while Eidetic required it. The old fixture invented a timestamp. A producer-shaped regression first failed, then passed after identity was bound to the producer's monotonic serving cursor, instance/service generations and actual metadata. Pre/post inference checks remain enforced. Same-model reloads with unchanged metadata and changes during embedding reject old vectors; unrelated serving mutations conservatively require Reindex.Exact-head local checks pass: 123 core + 615 server tests and all example targets; 16 targeted Pumas fixture tests; strict all-target/all-feature core/server Clippy; frontend lint/format/typecheck and 608 tests; ONNX no-download and authoring regressions; formatting and decision traceability. Independent review reports no remaining confirmed P1/P2 in the reviewed Pumas/reference paths. Exact-head CI run 100 completed successfully: Linux and Windows full-workspace Clippy/tests, frontend, ONNX no-download safeguards, authoring regressions and decision traceability all pass. Linux passed 983 Rust tests and Windows passed 981, with two existing native-display tests ignored on each platform. GitHub's PR merge tree matches the pushed head's tree. The local canonical workspace command remains blocked at missing glib/GTK/WebKit; its pre-push test hook was excluded after the listed checks, retaining hosted native CI.
Keep draft for parent review. Trained-model acceptance remains unexecuted: no local weights are present, and ordinary public artifact access receives proxy CONNECT 403; no bypass or credentials were used. Fixtures are synthetic test data only. The separate dependency audit also fails on the existing frontend stack (14 high / 3 critical, manifests and lockfile unchanged from main); local Rust audit tools are absent. Source CI and PR30's native UI captures do not satisfy those gates.
The committed readiness report records evidence and the deferred borrowed-bootstrap robustness observation. Minimal operator step: supply accessible local trained chat and embedding packages to the pinned Pumas runtime, load both, and run the real-model acceptance procedure. No release or merge has been performed.
Dependency audit repair — current head be03aa1
Current head:
be03aa1f2b7e6e19da632fac3b2b7eb89ec12c73, preserving1148fb0e. The exact-lockfile audit report maps all 14 high / 3 critical baseline package rows to 19 unique upstream advisories, actual shipped/runtime-capable versus build/lint/test consumers, affected ranges and repairs. The earlier inference from npmfixAvailable: falseto no upstream fix is corrected.Targeted compatible patches include Vite 6.4.4 and the affected devalue/PostCSS/nanoid/source-map/YAML/brace chains. Vitest and coverage move together to 4.1.11, removing unpatched Tinypool 1.x through its supported parent rather than forcing a transitive major. Kit 2.70.3 also repairs a verified reachable development-page Accept-header path; humanfs 0.16.8 and selector-parser 7.1.6 close the related ESLint/PostCSS lower-severity chains. No unrelated direct Svelte/ESLint/Tailwind/TypeScript/Rust family update, overrides or audit-force command is used.
Final local audit: zero moderate/high/critical; three low rows are one cookie 0.6 advisory propagated through Kit and adapter-static. Kit 2's current supported range still requires cookie 0.6; the report records absent application cookie-metadata input/Node production server, owner, reassessment triggers, and the parent decision to schedule a separately reviewed Kit 3 migration or supported backport. This is a specific existing low finding, not a blanket acceptance or reduced gate.
Fresh npm ci, npm ls, dependency script, lint/format/typecheck, all 107 files / 608 frontend tests and static build pass. A bounded synthetic Buffer regression reproduces devalue 5.8.1 serializing a 64-byte backing store for a two-byte view; patched 5.9.4 preserves only the selected two bytes. A five-test V8-coverage smoke passes. No tests, assertions or discovery filters were weakened. Independent GPT-6.1 Sol High review approved this exact commit with no confirmed P1/P2. Exact-head CI run 101 completed successfully: frontend audit/lint/format/typecheck, all 608 tests and static build; decision traceability; Linux/Windows full-workspace Clippy and tests; ONNX no-download safeguards. Linux passed 983 Rust tests and Windows passed 981, retaining two existing native-display tests ignored on each. Hosted audit confirms only the three low rows for the single cookie advisory. The CI merge tree matches the pushed candidate.
PR29 remains draft and unmerged for parent review. Trained-model inference remains unexecuted and gated; no model-access bypass, credentials or release action occurred. Local native testing remains environment-blocked at glib/GTK/WebKit, and optional local Rust audit tools remain absent as explicitly reported.