Repository navigation
Conversation
…point at modules with headroom (#16178) The hook read each changed file whole. So editing one unrelated line of api/auth.py for #15757 made the commit answerable for a local BaseModel that was on base at :240 and belonged to another commit, and the instruction it printed named seven schemas_*.py modules that are all frozen at their size ceiling. The rule and its remedy could not both be satisfied. Scoping. The hook takes --changed-lines-only. With no --base it reads the staged diff: the local commit stage, which is where #15757 was blocked and which #13950's CI-only precedent deliberately left whole-file. With --base R it reads R..HEAD, for the PR stage. A model counts only if its class line was added. A pre-existing model is listed as "not counted" instead of hidden. A git failure fails the run instead of reading as "added nothing". The staged/range hunk parsing is added_lines() in tools/lint/_scan_helpers.py, next to the git plumbing that already runs with a scrubbed environment. No Python hunk parser existed anywhere in the repo. Wrapper defect. check-pre-commit-hook-pr.sh accepted --changed-lines-only together with --python and silently dropped it: validator mode never scoped, so a caller asking for scoping got a whole-file run with no sign of it. It now passes --changed-lines-only --base <resolved base> to the validator. A validator that cannot scope rejects the unknown flag, which is loud. Guidance. The fixed list of destinations is replaced by every schemas_*.py module's headroom, computed from the size guard's own MAX_LINES, KNOWN_LARGE and count_lines loaded by path, never copied. Frozen modules are marked: add a companion schemas_<domain>_<topic>.py instead. That is the owner's ruling on AC3, and the hook's docstring now records it as the convention. Tests. The scoping is driven through run() against a real throwaway git repo, because what is under test is the diff: an unrelated staged edit passes, a newly added model fails, the same pair holds in --base mode, and a missing repository fails closed. The wrapper's pass-through tests live in a companion file, because check-pre-commit-hook-pr_test.py is at its 747-line ceiling; its repo helper is loaded by path, not copied. Without --changed-lines-only the hook behaves exactly as before apart from the message. The .pre-commit-config.yaml and code-quality.yml changes that switch it on are held back pending the coordinator's OK, since both are shared files outside the scope boundary set for this issue.
…ommit and PR stage (#16178) .pre-commit-config.yaml gives the no-local-schemas entry --changed-lines-only, so the commit stage judges only the models a commit added, read from the staged diff. That is the stage where #15757 was blocked. A direct run of the hook still reads whole files. code-quality.yml gets the hook's first CI step, through the wrapper with --changed-lines-only --python. The wrapper passes the base it resolved and the hook scopes itself. Before this the hook never ran in CI at all. Path-filter coverage was checked rather than assumed. The step sits in the code-quality job, which is gated by code-quality-paths.yml's backend filter. That filter already covers everything the step reads: **/*.py (the hook, _scan_helpers, the size guard and its ceiling data, api/), pipeline-scripts/** (the wrapper), tools/lint/**, and scripts/lib/** (the git-scope.sh the wrapper sources). No python_filter_uncovered_reads.py record is needed: that record covers repo_tests guards, and this adds none. Both files were approved by the coordinator for this issue. They are shared files, and no other in-flight branch touches either.
…it carries (#16178) added_lines() diffed only the file being checked, and a diff limited to one path cannot pair a rename with its source. A `git mv` of an API file therefore read as wholly new, and every pre-existing model in it was reported as introduced by the change. That broke #16178's first criterion: a pre-existing violation must not fail a change that did not introduce it. The rename source is now found from the whole change (--name-status -M -z, so paths containing spaces or quotes survive). The pair is then diffed together, so a move adds only the lines it actually changed. Below git's rename-similarity threshold the file still reads as new, which at that point is what it is. The two git calls share _git_diff(), which runs in the scrubbed environment and raises on failure, so a scoped run still fails closed and never reads a git error as "nothing added". Tests: a pure move passes; a move that also adds a model still fails, so following the rename cannot hide one; and the same rename holds in a committed range.
…-ref and --all-files (#16178) enforce-precommit.yml runs every pre-commit hook in CI: with --from-ref HEAD^1 --to-ref HEAD^2 on pull requests, and --all-files on pushes. Findings there are warnings only. Neither mode stages anything. So the --changed-lines-only this PR added to the hook's pre-commit entry would have diffed an empty staged change in both, and reported every model, new ones included, as not counted. That is a scoped run passing everything against nothing, the failure this PR exists to remove. The blocking code-quality step passes --base and still caught new models, so the damage was limited to the warning-only job. It was still wrong. Two rules close it: - A base pre-commit provides is used. It exports the range as PRE_COMMIT_FROM_REF (pre-commit 4.6.0, commands/run.py:391), so the hook diffs FROM_REF..HEAD, which describes the checked-out tree it reads. An explicit --base still wins. - In staged mode, a file with no staged change at all is not part of the commit, so there is no change to scope it to. It is judged whole-file and reported as such, never as "nothing added". staged_paths() in _scan_helpers answers that with one git call, through the same scrubbed _git_diff() the rest of the scoping uses. Found from #16250, which noted that enforce-precommit runs every hook. That also means this PR's claim that the hook never ran in CI was false: it always ran there as a warning. I had searched the workflows for the hook's name, and a job that runs all hooks names none. The code-quality.yml comment is corrected to say it had no *blocking* CI step. Tests: an unstaged file is judged whole and says so; PRE_COMMIT_FROM_REF becomes the base, an explicit --base overrides it, and with neither the staged diff is used.
…hemas-changed-lines
…t it (#16178) Review nit on #16241, applied as the reviewer specified it. _resolve_base trusted any PRE_COMMIT_FROM_REF in the environment, so a value left exported in a developer's shell would re-scope a plain run to a stale range. pre-commit always sets PRE_COMMIT=1 for its hooks, and the range is now read only when that is present. An explicit --base still wins. The failure it closes was 'scoped to the wrong range', not a silent pass: a git error on a bad ref still fails closed. It is cheap enough to close anyway. Test: a PRE_COMMIT_FROM_REF with no PRE_COMMIT=1 is ignored.
…d refuse writes under a lapsed claim (#15950) AC4 and part of AC6, per the owner's rulings on #15950. AC4: progress-gated renewal. A run that was alive but hung renewed its claims forever, because "alive" only meant "the coroutine has not returned". Renewal now stops once the run has reported no progress for the stall window, the run is marked lapsed, and its claim expires at TTL. A renew that finds the claim already gone lapses it the same way. Progress is recorded at checkpoints every run passes through, and none of them is in a frozen file: - BaseProvider.chat_completion, where every provider the registry builds finishes a model call. Ollama and vLLM reach it too, through the ollama_provider and vllm_base subclasses that wrap the standalone classes. services/llm_service.py (1263/1263) and llm_shared/provider_registry.py (788/788) are both at their size ceilings and are not touched. - ToolSDKRegistry.execute, after every SDK tool run. - require_held, since a write that passes the check is progress. The legacy tool handler table inside the frozen tool_registry.py is not instrumented. An agent loop still returns to the model each turn, so only a single legacy tool call longer than the whole window can lapse a working run. The window is max(AUTOBOT_WORK_CLAIM_STALL_INTERVALS x renewal interval, 2 x AUTOBOT_LLM_REQUEST_TIMEOUT). It is tied to the configured timeout, not a number, because a model call reports progress only when it finishes: a window shorter than one call would lapse a run doing exactly what it should. The per-run record is autobot_shared/coordination/run_progress.py, a ContextVar. That makes it reachable from the provider and tool layers, which must not import from agents. Tasks created inside a run copy the context, and nested runs pass progress outward. AC6, partly: require_held() at a declared write site, per the owner's table: held -> allow; lapsed or lost -> refuse (ClaimNotHeld); degraded (registry unavailable) -> allow and log; no claimed run at all -> allow with a named warning. The last row exists because only the agent_client path holds declared scopes. workflow_runner and the LLC adapter call process_request directly (#16269). Refusing there would break live writes, and the warnings measure which paths really carry them. The KB librarian's add_knowledge, the only declaring write site, calls it first. Also corrects the module docstring's claim that cancelling the renewer in `finally` frees a hung executor's scope. A hung run never reaches `finally`, which is why AC4 exists. Tests: a progressing run keeps its claim; a silent one lapses and stops renewing; progress from a spawned task and from a nested run reaches the outer run; the require_held table row by row; and the guard AC6 names. The KB librarian refuses add_knowledge under a lapsed claim and writes under a live one. AUTOBOT_WORK_CLAIM_STALL_INTERVALS is registered, and its CLAUDE_RULES.md row matches the generator's format (count 219 -> 220). AC1 is blocked by #16173 (NPU's file is at its ceiling), AC3 by #16174, and the rest of AC6 by #16269.
…bound the stall window by the token wait too (#15950)
…e summary, direct-mode and fail-closed tests (#16191)
…ory root, where it would pass unexamined (#16191)
…es them per node (#16280, #16281) The agent's heartbeat carries extra_data["gpu"] from autobot_shared.gpu_telemetry: absent for an older agent, [] for none present, or devices each measured or marked unmonitored. hardware_acceleration.py moves onto the same parser. The SLM serves GET /api/monitoring/gpu/nodes and publishes per-node autobot_gpu_* series from Node mapper events; the shared GPU gauges gain a node label that defaults to the local host.
…s from (#16281) Route tests drive the real router for the three states and refuse an anonymous request through the route's own get_current_user. Two source checks pin the mount-level guard: api/__init__.py mounts the GPU router on monitoring_router, and main.py includes that router with dependencies=_SM (require_service_management). The api/__init__.py comment now names that include-level dependency as the guard's source.
Auto-regenerated (py3.14) to match the backend and/or SLM backend schema so the required verify-generated-types gate(s) pass. Triggered by auto-fix-generated-types.yml.
…telemetry (#16289) Both parsed nvidia-smi CSV by hand. worker_node's int() of an unreadable "[N/A]" raised out of _get_nvidia_smi_details; such a GPU is now skipped. diagnostics split the whole output on ", ", so a second GPU folded into the first and reported an error; it now reports the first GPU. subprocess is no longer imported by either module.
…070 (#16289) HardwareDetector.check_gpu_availability and HardwarePerformanceMonitor._check_gpu_availability answered True only when nvidia-smi's name contained "RTX 4070", so GPU metrics were off on every other NVIDIA card. Both now ask autobot_shared.gpu_telemetry whether nvidia-smi reports any GPU. Each is pinned both ways: a non-4070 card is available, and no GPU is not.
…telemetry (#16289) GPUCollector and HardwarePerformanceMonitor.collect_gpu_metrics each split nvidia-smi's whole output on commas, so a second GPU shared the first one's split. Both now take rows from autobot_shared.gpu_telemetry (METRICS_QUERY_FIELDS, off the event loop via asyncio.to_thread) and still return the first GPU; every GPU reaching the consumers is #16297. A two-GPU test per collector, with the second GPU different in every field, pins that nothing of the second row leaks in. hardware_metrics.py drops from 1550 to 1513 lines and no longer imports subprocess.
…h gpu_telemetry (#16289) _check_nvidia_gpu, _check_amd_gpu and _detect_nvidia_capabilities now run the tools through autobot_shared.gpu_telemetry. _detect_amd_capabilities reads rocm-smi's JSON by key through parse_rocm_smi_json instead of scraping its text. The tests mock the new seams: a missing, failing or hung tool reaches these functions as None, and gpu_telemetry_test covers each of those. The NVIDIA capability probe now uses AUTOBOT_GPU_PROBE_TIMEOUT_S (5 s) instead of 10 s. test_gpu_detection.py drops from 763 to 718 lines.
…s unlike the dev host (#16289) TestGuiAutomation pops sys.modules["worker_node"] and re-imports it, so patch("worker_node.query_nvidia_gpus") reached the new module while the file's WorkerNode ran with the old one's globals, calling the real nvidia-smi. The helper now resolves the module at call time and patches that object, and asserts the seam was called. The fixtures borrowed this host's GPU name and memory, which let a leaked real call pass for fixture data; they are now invented.
…ys when it can't (#15226) usePrometheusMetrics.fetchGPUDetails read no endpoint and returned a literal "GPU absent, 0%". It now reads GET /api/monitoring/gpu/nodes, keeps each node's state in gpuNodes, and leaves the fleet summary null rather than zeroed when nothing is measured. A NodeGpuSummary row on each node card in the details view shows the three states distinctly -- not reported, none, present (unmonitored or measured) -- plus unavailable when the read fails. fetchRecommendations reports null (unavailable) instead of fabricating an empty list, and AlertsMonitor says so. NPUDeviceType is now the generated schema type rather than a hand-kept copy.
…hemas-changed-lines
… guard can resolve them (#16191)
…hemas-changed-lines
…e's agent copy (#16280) The canonical slm/agent/health_collector.py and heartbeat_payload.py changed, and heartbeat_gpu_16280_test.py was added, without updating the role's byte-identical mirror in ansible/roles/slm_agent/files/slm/agent/. The code-quality drift check (diff -r) went red. With all three copied, the two trees match again under the check's own exclusions.
… gpu_telemetry (#16289) HardwarePerformanceMonitor._gpu_metrics_from_row and GPUCollector._build_metrics were the same 30 lines, and each file had its own _gpu_int, which added 25 duplicated lines to the main scope's duplication count. The mapping is now gpu_telemetry.nvidia_metric_fields (plus parse_nvidia_int), with unchanged behaviour and the same throttle meaning. Each collector adds its timestamp and builds its own GPUMetrics. The new helpers have unit tests, and hardware_metrics.py's shrink (1513 -> 1478) is recorded in both size baselines.
…ct, not an inline generic (#15226) `slmApiClient.get<GPUNodeListResponse>(...)` was a shape assertion TypeScript couldn't check, and it took the SLM inline-generics ratchet from 87 to 88. - @autobot/ui gains the GetPath / JsonOf / GetResponse types. They're generic over a `paths` contract, so either frontend's client can use them without a fork. - SlmApiClient.getContract(path) accepts only a GET path from the generated `paths`, and returns that path's 200 schema. It strips the contract key's /api prefix, because the base supplies it. - usePrometheusMetrics reads /api/monitoring/gpu/nodes through it. - Tests: runtime prefix handling, plus type checks whose @ts-expect-error lines fail vue-tsc if a wrong path or response type were accepted.
# Conflicts: # autobot-backend/tests/agents/test_scope_enforcement.py
…'s line numbers, so large commits and base merges commit in one pass (#16343) Upstream's detect-secrets manifest sets no require_serial, so pre-commit ran the hook in parallel batches that each rewrote .secrets.baseline and raced: the last writer won, and a large commit failed on every attempt. The baseline's line numbers were also stale on base after the train shifted lines in baselined files. require_serial: true fixes the race; the refresh changes only line_number fields (and generated_at). A guard pins require_serial on every hook that rewrites a shared file.
Contributor
|
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: trueThanks 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 |
Contributor
Owner
Author
|
Train-g went red on |
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test vehicle, do not merge. This PR runs CI once over the combined state of the train's PRs. It is closed once they have landed.
Thinking Path
This is the owner-approved manual merge train (#15937, comment 5632692837). Train-f (#16350) landed tree-equal: Dev_new_gui is
affea4b38a, treef1e48845ad. Train-g carries every candidate whose four-layer pre-flight passed at its current head and that merges cleanly with the others.git merge-treeagainst base shows the pairs that share files (.pre-commit-config.yaml: #16349 and #16241;docs/developer/CLAUDE_RULES.md: #16292 and #16273) merging without conflict.What Changed
Branch
train/2026-09-11-g, built fromDev_new_guiataffea4b38athrough the repository merges API. All 4 merged with HTTP 201. Tip8430ad2f7c.e00a5037e2ea864bd54203cf17e0efee4a16a88dNot on this train:
.secrets.baselinediffers from #16349's only ingenerated_at, which is still a 409. It merges base and takes base's baseline after #16349 lands.secrets.baselineconflicts with #16349 in 603 hunks (line_numberstrip vs refresh). It regenerates after #16349 landsFullTreeDriftPanel.vueVerification
This PR's own CI run is the verification. The merge burst refuses to start unless the four-layer pre-flight passes on this tip and base is still
affea4b38a. It then:--match-head-commitand an explicit trailer-free message;Dev_new_gui's tree to equal this tip's tree.Nothing was run locally.
Model Used
Claude Opus 5 (the merge-owner session).
Refs #15937
Refs #16349
Refs #16292
Refs #16273
Refs #16241
🤖 Generated with Claude Code