From 12eef8e88507fe6037f4980be34c8a0ac2554c89 Mon Sep 17 00:00:00 2001 From: Hongsheng Liu Date: Wed, 14 Oct 2026 22:20:09 +0800 Subject: [PATCH 1/3] docs: complete repository loop entrypoints lessons and manual handoff --- .agents/skills/precheck-pr/SKILL.md | 5 + .agents/skills/self-review/SKILL.md | 5 + .agents/skills/system1-omni-review/SKILL.md | 5 + .../skills/write-optimization-blog/SKILL.md | 5 + AGENTS.md | 39 ++++++++ CONTRIBUTING.md | 5 + README.md | 4 + docs/development-loop.md | 91 +++++++++++++++++++ lessons.md | 47 ++++++++++ mkdocs.yml | 1 + 10 files changed, 207 insertions(+) create mode 100644 AGENTS.md create mode 100644 docs/development-loop.md create mode 100644 lessons.md diff --git a/.agents/skills/precheck-pr/SKILL.md b/.agents/skills/precheck-pr/SKILL.md index a5a0ca1e..34ce4fcf 100644 --- a/.agents/skills/precheck-pr/SKILL.md +++ b/.agents/skills/precheck-pr/SKILL.md @@ -5,6 +5,11 @@ description: Self-review a System1-Omni branch before opening a PR or marking it # PR self-review +Read the root [agent entrypoint](../../../AGENTS.md) and applicable +[lessons](../../../lessons.md) before this review. Use the canonical +[development loop](../../../docs/development-loop.md) for evidence freshness and +handoff; independent read-only reviews remain outside the contributor-intent gate. + Review the contributor's changes against System1-Omni's conventions. Read `CONTRIBUTING.md`, `README.md`, and applicable `AGENTS.md` instructions from the checkout before starting. Paths below are relative to the repository root. diff --git a/.agents/skills/self-review/SKILL.md b/.agents/skills/self-review/SKILL.md index 1cbd7aa4..fb461b09 100644 --- a/.agents/skills/self-review/SKILL.md +++ b/.agents/skills/self-review/SKILL.md @@ -5,6 +5,11 @@ description: Self-review System1-Omni changes before requesting maintainer revie # System1-Omni self-review +Read the root [agent entrypoint](../../../AGENTS.md) and applicable +[lessons](../../../lessons.md) before this review. Use the canonical +[development loop](../../../docs/development-loop.md) for evidence freshness and +handoff; independent read-only reviews remain outside the contributor-intent gate. + Start with the [human contributor-intent gate](#human-contributor-intent-gate). Follow the target checkout's `CONTRIBUTING.md` when present and its PR template. diff --git a/.agents/skills/system1-omni-review/SKILL.md b/.agents/skills/system1-omni-review/SKILL.md index 392df4c8..066e3ce9 100644 --- a/.agents/skills/system1-omni-review/SKILL.md +++ b/.agents/skills/system1-omni-review/SKILL.md @@ -5,6 +5,11 @@ description: Review ThinkFlowLab/system1-omni pull requests for Rust serving, mo # System1-Omni PR review +Read the root [agent entrypoint](../../../AGENTS.md) and applicable +[lessons](../../../lessons.md) before this review. Use the canonical +[development loop](../../../docs/development-loop.md) for evidence freshness and +handoff; independent read-only reviews remain outside the contributor-intent gate. + Use in `ThinkFlowLab/system1-omni` (repository ID `1386454853`). Produce a local reviewer report. The existing `precheck-pr` and `self-review` skills support contributor preparation; this skill supplies repository-specific independent review and does not replace their checklist. It grants no authority to edit code, push, post reviews/comments, approve, change labels or merge. It does not authorize model downloads, GPU reservations, paid execution or benchmark campaigns. ## Establish evidence diff --git a/.agents/skills/write-optimization-blog/SKILL.md b/.agents/skills/write-optimization-blog/SKILL.md index 8feb36bf..ff4d0b4e 100644 --- a/.agents/skills/write-optimization-blog/SKILL.md +++ b/.agents/skills/write-optimization-blog/SKILL.md @@ -9,6 +9,11 @@ Turn implementation and experiment records into a readable engineering post. Explain the bottleneck, the change that addresses it, and the measured result. Keep measured behavior distinguishable from implemented support and future work. +When the destination is System1-Omni, first read its root +[AGENTS.md](../../../AGENTS.md), [CONTRIBUTING.md](../../../CONTRIBUTING.md), +and applicable [lessons](../../../lessons.md). For another repository, use that +repository's own instructions and canonical lessons. + ## Choose the destination and evidence Inspect the target repository's blog/docs conventions and contribution rules. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..02a07a85 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,39 @@ +# Working in System1-Omni + +Before implementation or contributor self-review, read [CONTRIBUTING.md](CONTRIBUTING.md), +including its human contributor-intent gate. Before changing or reviewing an area, +consult [lessons.md](lessons.md) and the relevant contracts below. Follow any more +specific instructions in the directory you are working in; report conflicts. +Independent read-only review stays outside the contributor-intent gate. + +## Find the source of truth + +- Setup and entrypoints: [README](README.md) and [first CPU decision](docs/getting-started.md). +- Ownership, implemented versus planned layers, numerical and lifetime invariants: + [architecture contracts](docs/architecture.md). +- Commands, platforms and validation handoffs: + [change-to-validation matrix](CONTRIBUTING.md#choose-validation-by-changed-area). +- Contributor self-review and independent maintainer review: + [contribution process](CONTRIBUTING.md#self-review-before-requesting-review). +- Resuming work, evidence freshness, safe retries and measurements: + [development loop](docs/development-loop.md). +- Preventing repeated mistakes and recording verified incidents: [lessons](lessons.md). + +## Navigate the implementation + +Transport lives in `src/frontend/`; admission and shared execution interfaces in +`src/runtime/`; model-specific processing/execution in `src/models/`; hardware +operations in `src/backends/`. Read the affected component README and model +recipe before editing. New tests belong under root `tests/`; preserve explicit +Cargo target wiring and the test-module wiring rules in CONTRIBUTING. + +Use the relevant repository skill deliberately: [add-new-model](.agents/skills/add-new-model/SKILL.md), +[precheck-pr](.agents/skills/precheck-pr/SKILL.md), +[self-review](.agents/skills/self-review/SKILL.md), or +[independent review](.agents/skills/system1-omni-review/SKILL.md). +Their presence does not guarantee automatic invocation by any tool. + +Keep code, contracts and user-facing documentation aligned in the same change. +Record exact revisions, personally run commands, observed CI and unverified +hardware separately. These instructions grant no permission to publish, merge, +change settings, install monitoring, or start paid/GPU runs. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 77418c24..63ae2afd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,6 +4,11 @@ Review the [README](README.md) and [architecture and integration contracts](docs/architecture.md) for the project's ownership boundaries and current implementation status before making changes. +Before changing or reviewing an area, read the applicable [lessons](lessons.md). +Use the [development loop](docs/development-loop.md) to checkpoint evidence and +resume work. Agent-assisted contributors also follow [AGENTS.md](AGENTS.md); +these entrypoints link to the same canonical guidance. + ## Before implementation or a PR Complete the [human contributor-intent gate](.agents/skills/self-review/SKILL.md#human-contributor-intent-gate) diff --git a/README.md b/README.md index e4316082..a12135ba 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,10 @@ request: self-review the full diff, keep serving, processing, scheduling, and model execution separate according to the [architecture contracts](docs/architecture.md), and run the checks appropriate to your changes. +For agent-assisted work, start with [AGENTS.md](AGENTS.md). The +[development loop](docs/development-loop.md) explains resumable handoffs, and +[lessons](lessons.md) records verified prevention rules. + **Have an idea or found a problem?** [Open an issue](https://github.com/ThinkFlowLab/system1-omni/issues/new) with the details, or [send a pull diff --git a/docs/development-loop.md b/docs/development-loop.md new file mode 100644 index 00000000..cac468ab --- /dev/null +++ b/docs/development-loop.md @@ -0,0 +1,91 @@ +# Development loop and handoff + +This is a contributor-operated workflow using existing GitHub issues, PR bodies, +comments and check links. It defines no installed watcher, background runner, +webhook, hook or new permission. Automation deployment and its owner, storage, +concurrency controls and delivery verification remain a separate decision. + +Start from [CONTRIBUTING](../CONTRIBUTING.md); agents also read +[AGENTS.md](../AGENTS.md). Consult [lessons](../lessons.md) before changing or +reviewing an area. Architecture and validation stay in their +[canonical contracts](architecture.md) and +[change-to-validation matrix](../CONTRIBUTING.md#choose-validation-by-changed-area). + +## One change, one evidence trail + +1. Complete the existing human intent gate, define the bounded change and select + validation from the matrix. Record affected components and prerequisites. +2. Implement with tests and contract/documentation updates. Capture revision, + platform/tool versions, exact command, start/end time, result and evidence URL. +3. Self-review the complete diff and fill the existing PR template. A green CPU + suite does not validate GPU behavior; author evidence does not replace + maintainer review. Keep blocked validation and its handoff owner visible. +4. Before pausing or handing off, update a single **Handoff checkpoint** in the + authorized PR body or issue. Read it back to verify persistence. For work + without a remote item, retain the same fields in the contributor's private + task workspace until publication is authorized; do not commit run state. +5. On resumption, read the checkpoint and current remote state before choosing the + next action. Preserve past results as history, clearly separated from evidence + applicable to the current head. A closed issue or merged PR does not complete + every dependency automatically. + +## Handoff checkpoint fields + +Use the existing item as the durable record; do not open a duplicate tracker. +The following fields can be plain text in a PR body/comment or issue: + +- Repository identity and item number/URL; observation time and sources. +- Open/closed/merged state; actual PR base/head SHA. Issues have no inherent head; + link their relevant PRs and revisions instead. +- Each relevant check's name, run/attempt, checked SHA, status/conclusion, + platform, observation time and evidence link. Keep unknown/stale separate. +- Review/requirement status and validation owners, independently of author test + results. Unknown branch-protection or merge authority remains unknown. +- Dependencies, blockers, next action and responsible person, or explicitly + unassigned handoff. Include what evidence will unblock it. +- For an authorized external action: a stable transition/target key, purpose, + planned/in-flight/confirmed/uncertain state and returned external identifier. + Preserve enough history to avoid replaying it after a restart. + +An action key includes repository/item, action type and meaningful evidence +version (for example head + check run/attempt + conclusion), not only the head. +Read before editing a shared record; coordinate concurrent writers. If a write's +result is uncertain, inspect the destination before retrying. A deduplication +record is not a guarantee of exactly-once delivery or authorization to act. + +## Reconcile changes and demonstrate the protocol + +This synthetic walkthrough exercises the rules, not a deployed tracker: + +| Observation | Checkpoint / next action | +| --- | --- | +| Head A, run 10 pending | Save A/run 10 and the pending validation handoff; read it back after a pause. | +| Same A/run 10 failed | Refresh CI evidence despite unchanged head; record a new failure transition and diagnosis handoff. | +| Identical failure observed again | Retain the existing transition key; do not duplicate an action or notification. | +| Same A, rerun 11 succeeds | Record the new attempt and supersede current CI status; do not erase history or infer human approval. | +| Head B replaces A | A's check evidence is historical; select and obtain B's checks. Reassess review applicability under actual repository policy. | +| Base changes while head stays B | Reassess merge-result validation; refresh reviews, requirements and mergeability independently. | + +Before a separately authorized merge or other consequential action, refresh the +actual head and relevant gates. Check runs and legacy commit statuses are separate +sources: an empty status list is not a failing check. If a source cannot be read, +record the last known result and its age without calling it current. + +## Bounded execution and useful measurements + +Choose a time/attempt budget before expensive work. Retry only safe transient +failures within that budget; a retry cannot expand authorization. Diagnose +deterministic failures instead of repeatedly running them. Stop on access denial, +exhausted budget, scope change or missing human decision, and checkpoint the +blocker. Do not run parallel jobs against conflicting mutable resources. + +Use current CI timeouts and the [benchmark protocol](../benchmarks/README.md) +where applicable; GPU reservations, downloads and paid runs require their own +scope and resources. Record failures and skipped/ignored tests honestly. + +For an iteration improvement claim, define the observation window and metric +before collecting it. Examples are elapsed time from first submitted revision to +first actionable validation result, or PR open-to-merge time. Record timestamps, +data sources, sample count, exclusions and unfinished items. Count independent +incidents using [lessons](../lessons.md), not alerts. No baseline or speedup is +claimed by this initialization; one successful PR cannot establish causality. diff --git a/lessons.md b/lessons.md new file mode 100644 index 00000000..c2d180c9 --- /dev/null +++ b/lessons.md @@ -0,0 +1,47 @@ +# Lessons + +Read relevant active lessons before changing or reviewing an area. After a verified +failure or review correction, propose a concise prevention rule in the same PR or +its explicit follow-up. Keep normal architecture and review policy in their +canonical documents; this file records evidence-backed learning. + +## Active lessons + +No verified lessons recorded yet. This initialization does not infer historical +incidents from old PRs or count its acceptance examples as real failures. + +## Entry format + +For each actual lesson, record: + +- Stable lesson ID, affected scope, and concise prevention rule. +- Incident key and reviewer-accessible source link; observed failure separately + from suspected cause, with confidence when attribution is uncertain. +- Concrete preventive check and its canonical test/tool/document location. +- Named owner or explicitly unassigned next action, plus active/resolved status. +- Independent incident count and the distinct incident keys supporting it. + +Do not add empty incident entries to satisfy this format. Keep secrets, private +conversation links, personal data and one-off execution dumps out of the file. + +## Bounds, recurrence and archive + +The active file limit is 200 lines, including this guidance. + +- Keep this active file within that limit. Consolidate + duplicate lessons before archiving resolved or superseded detail. +- Count independent failures, not retries, repeated observations or duplicate + reports of one incident. Uncertain identity stays unresolved, not incremented. +- Proposed recurrence threshold for maintainer review: at two independently + evidenced incidents with the same failure mechanism, + propose a regression test, invariant, tooling check or design repair, with an + owner and reviewable next action. This threshold is a proposal, not permission + to change infrastructure or policy automatically. +- When there is material to archive, create `docs/lessons-archive/YYYY-MM.md`, + retain source/incident history, and leave a short active-file reference. + No archive is created until there is real content. Never hide an unresolved + risk to meet the line limit. + +Contributors propose updates through ordinary review; maintainers decide policy +changes. See [the development loop](docs/development-loop.md) for durable task +state and [CONTRIBUTING](CONTRIBUTING.md) for validation and review. diff --git a/mkdocs.yml b/mkdocs.yml index 055b7c24..b50d7bf1 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -77,3 +77,4 @@ nav: - LFM2.5-350M choice worker: recipe/lfm2/README.md - D3 reference worker: recipe/d3/reference.md - Contributing: CONTRIBUTING.md + - Development loop: docs/development-loop.md From 2184a17496c67daa39ad0edc101c5bbf3221344b Mon Sep 17 00:00:00 2001 From: Hongsheng Liu Date: Wed, 14 Oct 2026 22:32:17 +0800 Subject: [PATCH 2/3] docs: align runtime frontend and CUDA validation contracts with current code --- docs/architecture.md | 11 +++++++--- src/backends/cuda/contract.md | 18 +++++++++------- src/frontend/README.md | 39 ++++++++++++++++++++++++++--------- src/runtime/README.md | 20 +++++++++++++++--- 4 files changed, 65 insertions(+), 23 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 9fb69e11..e54fe2f1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -6,8 +6,11 @@ design. Concrete input/output types follow each executor's supported layout. ## Implementation status -The [Rust frontend](../src/frontend/README.md) currently forwards HTTP requests -to separately running workers. Cua-S1 and Open-Jev have native Rust/CUDA workers +The [Rust frontend](../src/frontend/README.md) binary forwards HTTP requests +to separately running workers. Its library also provides an in-process transport +for the runtime's byte-oriented `Engine` interface; admission, readiness and queue +reporting stay worker-owned. This interface does not implement shared model +processing or dynamic batching. Cua-S1 and Open-Jev have native Rust/CUDA workers that share the [Qwen3.5/3.8 executor](../src/models/qwen3_5/native/), which accepts single prompts and bounded packed prefill. Cua-S1 and Open-Jev-9B use single-prompt calls; Open-Jev-27B-v1.1 packs candidates within one request for @@ -118,7 +121,9 @@ executor, with FIFO waiting. Engines own scheduler instances; model-specific execution adapters declare the unit and submit owned closures. Waiting is async, so queued requests do not occupy blocking threads waiting for a model lock. Schedulers are local to each worker/executor; this does not coordinate separate -processes. Queue-length limits and token budgets are not implemented yet. +processes. `SerialScheduler` does not impose queue-length limits or token budgets. +The separate `Engine` interface can report a worker's capacity and refusals; it +does not itself implement an admission policy. - Batch only work accepted by the same loaded executor, with compatible checkpoint/adapter identity, device, dtype, and input layout. Models declare diff --git a/src/backends/cuda/contract.md b/src/backends/cuda/contract.md index 41301af1..c4f2c948 100644 --- a/src/backends/cuda/contract.md +++ b/src/backends/cuda/contract.md @@ -13,8 +13,11 @@ engine" is the older word for the second of those. A backend does not need identical internal structures to another backend, and no universal tensor abstraction is introduced here. -Status: proposed. The checker is [`check_contract.py`](check_contract.py); wiring it -into CI is a separate change, so nothing here is enforced yet. +Status: the static Tier 1 checker, [`check_contract.py`](check_contract.py), is +wired into the `cuda-contract` job in [CI](../../../.github/workflows/ci.yml), +alongside checker and backend tests. The Tier 2 CUDA compile and Tier 3 GPU parity +jobs described below are not configured there. Static enforcement does not prove +compiled artifacts or numerical parity. ## Why this exists @@ -205,7 +208,7 @@ instead of leaving it to be discovered in a parity failure: ## 4. Validation -Two tiers, because they have different requirements. +Three tiers, because they have different requirements. **Tier 1 — contract check (no GPU).** `check_contract.py` reads the manifests and the build scripts and checks: the schema, that every declared source exists, that @@ -213,7 +216,7 @@ a build script declares the architectures the manifest claims and writes the library the manifest names, that `abi_version` matches the `_ABI_VERSION` macro the declared sources define, and that a `validated` backend declares both a tolerance and a reference entrypoint. It runs on a stock runner and needs no CUDA -toolkit. This is the tier implemented in this change. +toolkit. This tier is implemented and runs in the current CI workflow. It does not compile CUDA and does not prove numerics. @@ -240,9 +243,10 @@ wired up. The split is the point. A CPU-only workflow that claims to validate CUDA is worse than no workflow, which is the gap `#14`'s own validation report already records: -today's CI checks every Rust feature and compiles no CUDA at all. Tier 1 removes -the part of that gap which does not need hardware; Tiers 2 and 3 need hardware and -are therefore separate. +the current Rust CI selects `omni-laya/serve`, and the CUDA-contract job reads +source text without compiling CUDA. Tier 1 covers the static part of that gap; +Tier 2 needs a CUDA toolchain, and Tier 3 additionally needs a GPU and model +artifacts. Neither is established by a passing Tier 1 job. ## Reporting diff --git a/src/frontend/README.md b/src/frontend/README.md index 4acb3a5e..edc7ab97 100644 --- a/src/frontend/README.md +++ b/src/frontend/README.md @@ -1,14 +1,16 @@ # Rust frontend -An Axum/Tokio server that forwards requests to a separately running model worker -using Reqwest. The worker handles validation, media loading, preprocessing and -inference. +The default `omni-jev` binary is an Axum/Tokio server that forwards requests to a +separately running model worker using Reqwest. The library also exposes an +in-process transport described below. In both paths the worker handles model +validation, media loading, preprocessing and inference. In the [target architecture](../../docs/architecture.md), a shared Rust worker runtime coordinates independent processors, a scheduler/batcher, and model executors. Rust handles host orchestration; CUDA/Metal handle device operations -through backend bindings. That shared runtime is planned; the current frontend -continues to forward requests to existing model-specific worker pipelines. +through backend bindings. Serial admission and an in-process `Engine` interface +exist; shared processing orchestration and dynamic batching remain planned. The +default binary continues to forward to existing model-specific worker pipelines. ## Run @@ -35,7 +37,7 @@ connections bypass system HTTP proxies. The health timeout and response limit must be positive integers. -## HTTP interface +## Forwarding HTTP interface - `POST /v1/systemone` forwards the `model`, `state` and `questions` envelope unchanged. Workers return `choice`, `score` or `noul` decisions; see the @@ -55,6 +57,23 @@ Text, image, audio, video and mixed payloads pass through as bytes. Actual infer support depends on the worker. The [Laya recipe](../../recipe/laya/README.md) verifies text decisions against a real backend. +## In-process library transport + +[`omni_jev::engine::app`](src/engine.rs) accepts an `Arc` and +`ServiceConfig`; [`Engine`](../runtime/src/engine.rs) lives in `omni-runtime`. +It exposes the same two route paths, but it is a separate library integration, +not a binary configuration switch. `src/main.rs` selects the forwarding router. + +The transport passes request/response bytes without parsing the decision envelope. +It owns body limits, the total request deadline and HTTP status mapping; the +worker owns admission, execution, readiness and optional queue reports. Defaults +are a 30-second total budget and 1 MiB request body limit. Invalid requests map to +400, oversized bodies to 413, busy/unavailable engines to 503, inference failures +to 500, and upload/queue/inference timeouts to 504. Health reads engine readiness +without waiting for inference; successful replies expose the reported queue depth. +`engine::serve` drains HTTP connections; stopping admission and joining worker +threads remain the caller's responsibility. + ## Checks From the repository root: @@ -63,12 +82,12 @@ From the repository root: # Focused CPU-only API integration suite. cargo test -p omni-jev --test frontend --locked -# Full workspace checks. -cargo fmt --all --check -cargo clippy --workspace --locked --all-targets -- -D warnings -cargo test --workspace --locked +# Other transport paths and the in-process interface. +cargo test -p omni-jev --test forwarding_limits --test in_process --locked ``` Tests use local mock workers; no model weights or GPU are needed. They cover multimodal byte preservation, authorization, connection reuse, large uploads, backend errors, timeouts, health and binary startup/shutdown. + +For full workspace checks, use the current [Rust CI commands](../../CONTRIBUTING.md#rust-ci-checks). diff --git a/src/runtime/README.md b/src/runtime/README.md index bd571b03..8d2df3ff 100644 --- a/src/runtime/README.md +++ b/src/runtime/README.md @@ -33,9 +33,23 @@ existing failure behavior. This implements serial admission per executor within a worker process. Admitted concurrency is one. Queue-length limits, token budgets, compatibility grouping, -shared processing orchestration and GPU batching remain planned. The Qwen -executor still accepts one prompt per forward; this scheduler does not pack -inputs or coordinate separately running workers. +shared processing orchestration and cross-request GPU batching remain planned +for this scheduler. The shared Qwen executor supports single prompts and bounded +packed prefill: Open-Jev-27B-v1.1's model adapter groups up to 16 candidates and +4096 tokens, with longer prompts run alone, while Open-Jev-9B uses single-prompt +calls. `SerialScheduler` does not pack those inputs or coordinate separately +running workers. See the [architecture contracts](../../docs/architecture.md) +for grouping and reconstruction ownership. + +## In-process engine interface + +The separate [`engine` module](src/engine.rs) defines a byte-oriented `Engine` +contract: readiness, request submission with a deadline, a reply channel and +optional queue/failure reporting. Implementations own admission and their queue; +the trait does not supply a queue or batching policy. The +[frontend library](../frontend/README.md#in-process-library-transport) maps this +interface onto HTTP. The default `omni-jev` binary still forwards to an external +HTTP worker; the interface alone does not migrate existing native workers. CPU tests cover FIFO serialization, queued cancellation, retention of admission and resources after dispatched cancellation, errors/panics, and independent From 845b77db6e120d3df18329bce8a378a2d2290f77 Mon Sep 17 00:00:00 2001 From: Hongsheng Liu Date: Wed, 14 Oct 2026 23:01:01 +0800 Subject: [PATCH 3/3] docs: scope validation and handoffs to repository needs and resources --- CONTRIBUTING.md | 10 +++++---- docs/development-loop.md | 41 ++++++++++++++++++++--------------- lessons.md | 9 ++++---- src/backends/cuda/contract.md | 27 ++++++++++++----------- 4 files changed, 48 insertions(+), 39 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 63ae2afd..eff21ff9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -102,11 +102,13 @@ from the repository root and record the exact revision and environment. | Frontend transport (`src/frontend/`) | `cargo test -p omni-jev --test frontend --locked`; include `cargo test -p omni-jev --test forwarding_limits --test in_process --locked` when those paths change. | Check request/response, failure and cancellation behavior against the [frontend contract](src/frontend/README.md), then run Rust CI. | | Native admission or execution interface (`src/runtime/`) | `cargo test --locked -p omni-runtime` | Check affected workers against the [runtime contract](src/runtime/README.md) and [architecture invariants](docs/architecture.md), then run Rust CI. | | Benchmark tooling (`benchmarks/`, `tests/benchmarks/`) | Install `benchmarks/requirements.txt`; run `python benchmarks/bench.py validate benchmarks/smoke.jsonl` and `python -m unittest discover -s tests/benchmarks -p 'test_*.py' -v`. | Follow the [benchmark protocol](benchmarks/README.md) for performance claims. Tooling smoke tests are not model-quality or throughput measurements. | -| CUDA contract or backend tooling | Use the `cuda-contract` job's current commands and prerequisites in [CI](.github/workflows/ci.yml). | Text-based contract checks do not execute kernels. Changed native/device behavior needs the affected model recipe's hardware and numerical evidence. | -| Model processing, execution, batching or backend behavior | Select tests under root `tests/` using the model's manifest, README and recipe. | Follow the [architecture contracts](docs/architecture.md), [model/hardware coverage](docs/supported-models.md) and model-specific validation recipe. Identify unsupported and untested configurations; arrange unavailable hardware checks with the maintainer. | +| CUDA contract or backend tooling | Use the `cuda-contract` job's current commands and prerequisites in [CI](.github/workflows/ci.yml). | This is a CPU/static job: it does not compile CUDA or execute kernels. Choose further checks for the changed behavior from the affected model recipe and available resources. | +| Model processing, execution, batching or backend behavior | Select tests under root `tests/` using the model's manifest, README and recipe. | Follow the [architecture contracts](docs/architecture.md), [model/hardware coverage](docs/supported-models.md) and model-specific validation recipe. Identify unsupported and untested configurations. For performance claims or concrete numerical/performance risk, use the [conditional comparison guidance](.agents/skills/self-review/SKILL.md#conditional-ab-checks). Report unavailable checks and their effect on the claim; agree additional hardware validation separately. | -The author supplies reproducible results and names any unresolved validation -handoff. Maintainer review remains separate from author evidence. Do not claim +The matrix distinguishes existing checks from change-specific validation choices; +it creates no new hardware gate or obligation to supply hardware. The author +reports reproducible results and unresolved validation limits. Maintainer review +remains separate from author evidence. Do not claim merge authority or required-check settings from this matrix or passing CI alone. Before handing off, record the PR head, commands, results and evidence links, remaining blockers, and next action. Recheck evidence after a head change; diff --git a/docs/development-loop.md b/docs/development-loop.md index cac468ab..2e575f57 100644 --- a/docs/development-loop.md +++ b/docs/development-loop.md @@ -15,24 +15,28 @@ reviewing an area. Architecture and validation stay in their 1. Complete the existing human intent gate, define the bounded change and select validation from the matrix. Record affected components and prerequisites. -2. Implement with tests and contract/documentation updates. Capture revision, +2. Implement the bounded change. Add or update tests when behavior changes and + update affected contracts/documentation. Capture revision, platform/tool versions, exact command, start/end time, result and evidence URL. 3. Self-review the complete diff and fill the existing PR template. A green CPU suite does not validate GPU behavior; author evidence does not replace maintainer review. Keep blocked validation and its handoff owner visible. -4. Before pausing or handing off, update a single **Handoff checkpoint** in the - authorized PR body or issue. Read it back to verify persistence. For work - without a remote item, retain the same fields in the contributor's private - task workspace until publication is authorized; do not commit run state. -5. On resumption, read the checkpoint and current remote state before choosing the +4. For multi-session work or an actual handoff, optionally keep a **Handoff + checkpoint** in the authorized PR body or issue. Reuse existing PR evidence + and record only fields needed to resume the work. If used, read it back to + verify persistence. Without a remote item, keep relevant state in the + contributor's private task workspace until publication is authorized; do not + commit run state. +5. On resumption, read any checkpoint and current remote state before choosing the next action. Preserve past results as history, clearly separated from evidence applicable to the current head. A closed issue or merged PR does not complete every dependency automatically. ## Handoff checkpoint fields -Use the existing item as the durable record; do not open a duplicate tracker. -The following fields can be plain text in a PR body/comment or issue: +When a checkpoint is useful, reuse the existing item rather than opening a +duplicate tracker. The following are optional fields to select for the actual +handoff, not a checklist required for every change: - Repository identity and item number/URL; observation time and sources. - Open/closed/merged state; actual PR base/head SHA. Issues have no inherent head; @@ -73,15 +77,18 @@ record the last known result and its age without calling it current. ## Bounded execution and useful measurements -Choose a time/attempt budget before expensive work. Retry only safe transient -failures within that budget; a retry cannot expand authorization. Diagnose -deterministic failures instead of repeatedly running them. Stop on access denial, -exhausted budget, scope change or missing human decision, and checkpoint the -blocker. Do not run parallel jobs against conflicting mutable resources. - -Use current CI timeouts and the [benchmark protocol](../benchmarks/README.md) -where applicable; GPU reservations, downloads and paid runs require their own -scope and resources. Record failures and skipped/ignored tests honestly. +For benchmarks or resource-intensive work, follow time/attempt limits when the +applicable recipe or agreed execution plan specifies them. Do not impose a new +budget on ordinary contribution work. Retry only safe transient failures within +any applicable limit; a retry cannot expand authorization. Diagnose deterministic +failures instead of repeatedly running them. Stop on access denial, exhausted +agreed resources, scope change or missing human decision, and report the blocker. +Do not run parallel jobs against conflicting mutable resources. + +CI timeouts apply to their configured jobs, not as universal local-test budgets. +Use the [benchmark protocol](../benchmarks/README.md) only where applicable; +GPU reservations, downloads and paid runs require separately agreed scope and +available resources. Record failures and skipped/ignored tests honestly. For an iteration improvement claim, define the observation window and metric before collecting it. Examples are elapsed time from first submitted revision to diff --git a/lessons.md b/lessons.md index c2d180c9..7ea8b218 100644 --- a/lessons.md +++ b/lessons.md @@ -32,11 +32,10 @@ The active file limit is 200 lines, including this guidance. duplicate lessons before archiving resolved or superseded detail. - Count independent failures, not retries, repeated observations or duplicate reports of one incident. Uncertain identity stays unresolved, not incremented. -- Proposed recurrence threshold for maintainer review: at two independently - evidenced incidents with the same failure mechanism, - propose a regression test, invariant, tooling check or design repair, with an - owner and reviewable next action. This threshold is a proposal, not permission - to change infrastructure or policy automatically. +- Use the observed failure mechanism and impact to propose an appropriate + preventive change for review. There is no default incident-count threshold or + automatic escalation; any new test or policy depends on repository needs and + available resources. - When there is material to archive, create `docs/lessons-archive/YYYY-MM.md`, retain source/incident history, and leave a short active-file reference. No archive is created until there is real content. Never hide an unresolved diff --git a/src/backends/cuda/contract.md b/src/backends/cuda/contract.md index c4f2c948..64322229 100644 --- a/src/backends/cuda/contract.md +++ b/src/backends/cuda/contract.md @@ -102,7 +102,8 @@ before its second consumer lands. ### Build script interface -`build.script` is invoked by the compile job as: +A future compile job could invoke `build.script` as follows; no such job is +configured in the current CI: ```sh ./ @@ -220,12 +221,12 @@ toolkit. This tier is implemented and runs in the current CI workflow. It does not compile CUDA and does not prove numerics. -**Tier 2 — compile (no GPU).** Building each backend with `nvcc` in a CUDA -container, so a kernel that stops compiling cannot land. The interface it relies -on is the one Tier 1 already checks: `./ -`, producing exactly the declared library. Not yet wired up; -it needs a container image and belongs with the rest of the CI wiring rather than -with the checker. +**Tier 2 — proposed compile check (no GPU; not configured).** A possible future +job would build selected backends with `nvcc` in a CUDA container. This is not +an active merge requirement. Its proposed interface is the one Tier 1 checks: +`./ `, producing exactly the declared +library. Adopting it would require an explicit repository decision, a suitable +toolchain image and an agreed scope; this document does not require its installation. Note what a build check does and does not prove. It shows the sources compile and that a file of the declared name appears. It does not prove the binary honoured @@ -234,12 +235,12 @@ and nothing reads the artifact to confirm which targets are inside. Inspecting that would need `cuobjdump`, which is a reasonable follow-up rather than something this change claims to do. -**Tier 3 — GPU parity (self-hosted).** Running each validated backend's -`reference.entrypoint` on a real GPU, and reporting kernel-level and end-to-end -parity separately. Contributors with a card can attach their own runner to their -own fork, which is how `#19` (RTX 6000 Ada, sm_89), `#12` (RTX 4090) and `#14` -(H800, sm_90a) can each be checked on the hardware they were measured on. Not yet -wired up. +**Tier 3 — proposed GPU parity check (not configured).** Where a specific change +and available hardware justify it, a contributor can run the affected model's +existing reference recipe and report kernel-level and end-to-end parity +separately. GPU CI would be a separate repository decision, not an obligation +to attach a runner or provide hardware. The recorded hardware in `#19`, `#12` +and `#14` does not establish current CI capacity. The split is the point. A CPU-only workflow that claims to validate CUDA is worse than no workflow, which is the gap `#14`'s own validation report already records: