Conversation
The team-wide part of CONTRIBUTING.md is a copy of templates/CONTRIBUTING.md in the data_team repository, kept in sync by the sync-style-guide skill. Four repos held byte-identical copies and classification_node had already drifted, so the copies were re-synced from one corrected template and the marker heading convention the skill expects is now documented in the vault. AGENTS.md gives an agent the project structure, the commands that actually work here and the traps specific to this repository. It points at CONTRIBUTING.md for the coding standards instead of repeating them a third time. CLAUDE.md imports it via @AGENTS.md, matching nicegui, zoe-app and data_team. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
learning_loop_node is public, but AGENTS.md named dfine_node and classification_node, which are not — neither appears anywhere else in the public tree. The dependency rule they illustrated holds without naming them, and "grep ../dfine_node" is advice no external contributor can follow anyway. Also drop the claim that the shared Linting section contradicts this repo: that section now covers repositories without a .pre-commit-config.yaml explicitly. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every detector node does the same three things with a model's raw output: drop low-confidence predictions, suppress overlapping boxes, and turn what survives into the loop's dataclasses. None of it depends on the model, yet each node repository had grown its own copy, and the copies had drifted: clip_point was byte-identical in three places, while clip_box existed in the same three plus a fourth that used a centre-based convention under the same name. Move that code here as detector/postprocess.py and detector/geometry.py, and give the centre-based variant its own name (clip_box_centered) so the two conventions can no longer be confused. The library also carried two containers for the same detections: a detector reports ImageMetadata, a trainer's auto-detection pass reports Detections. to_image_metadata and to_detections now build both from one routine, so the two paths clip and filter identically instead of diverging. The scalar arguments of non_max_suppression and post_process are keyword-only: transposing origin_h and origin_w was too easy to do. Add tests/unit, the first suite that runs without a Learning Loop, and gate the credential-dependent jobs on it in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every main.py in every node repository repeats the same twenty lines: read UVICORN_RELOAD, build the settings, construct the node, call uvicorn.run. Eight copies across three repositories, and they had drifted - dfine parses settings with configargparse so each one is both a flag and an environment variable, while the others hand-roll os.getenv plus asserts and hardcode the host and port. node_parser and run_node put that in one place. A setting is named after its flag: --conf-threshold reads CONF_THRESHOLD. That is what yolov5_node and classification_node already use, so adopting this renames nothing in the repositories that are actually deployed. --host and --port are the exception, reading NODE_HOST and NODE_PORT. The bare HOST already means the loop's address - every deployment sets it - and a node adopting that would hand it to uvicorn and fail to bind. dfine_node required a DFINE_TRAINER_ / DFINE_DETECTOR_ prefix, so it passes legacy_env_prefix and its prefixed names keep working with a warning naming the replacement. The logging.basicConfig call the trainers carry is not reproduced here - the library configures the root logger on import, so it has been a no-op. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three things every trainer needs, none of which depend on the training framework, all of which existed only inside dfine_node or in worse form elsewhere: iterator_cpu_bound runs a training generator in a spawned process. A trainer that trains in-process needs this - the event loop must stay responsive and CUDA state must stay out of the node - and it is pure stdlib. find_batch_size probes for the largest power-of-two batch that fits, around a fits predicate the trainer supplies. The search is pure arithmetic; only the predicate needs a framework, so nothing here imports torch. Three repositories had three different implementations of this: a real probe, an estimate parsed out of torchinfo's summary text, and a doubling loop catching bare RuntimeError. is_out_of_memory exists because of that last one - catching RuntimeError treats every crash as a full card. macro_f1 scores the confusion matrix the loop stores, which was implemented three times over two different data shapes. _get_executor_error_from_log gets a default implementation, because yolov5_node and classification_node had byte-identical copies of it. The PEP 695 type parameters the generator used are rewritten as TypeVar and ParamSpec: this library supports Python 3.10 and dfine_node targets 3.12. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
learning_loop_node/tests/ is excluded from the wheel, so nothing in it is
importable by a node repository. That is why the node repositories have almost
no tests: every one of them would have to reinvent TestingTrainerLogic, the
data_folder fixture and the condition poller before writing its first
assertion. dfine_node/trainer/tests/ is the only offline suite in any of them.
The reusable half now lives in learning_loop_node/testing/, a real package that
ships:
- helpers.py - condition, update_attributes, get_files_in_folder, unzip,
get_latest_model_id
- detections.py - get_dummy_detections, get_dummy_metadata
- fixtures.py - the data_folder and clear_loggers fixtures, as an opt-in pytest
plugin
- trainer.py - TestingTrainerLogic, create_active_training_file,
assert_training_state
- detector.py - TestingDetectorLogic, TestingDetectorFactory
tests/ keeps only the library's own suites and stays excluded, as before. A
node opts into the fixtures with
pytest_plugins = ['learning_loop_node.testing.fixtures']
which is deliberately not a pytest11 entry point: data_folder is autouse and
wipes a directory, so no project should get it merely by installing the
library. For the same reason testing/__init__.py does not import fixtures, and
the package stays importable without pytest.
The data_folder fixture existed in six copies, and two of them - mock_trainer's
and mock_detector's - had drifted to not create the folder they point at. They
all become the one that does.
Destructive fixtures now refuse to run against production. LoopCommunicator
reads environment_reader.host(default='learning-loop.ai') and run_tests.sh
sourced .env with no guard, so a missing LOOP_HOST aimed project creation and
deletion at real customer data. assert_not_production_loop() is called by every
fixture that generates or deletes a project, and run_tests.sh fails before
pytest starts. CI is unaffected - the workflows pin preview.learning-loop.ai.
assert_training_state loses a debug except-Exception branch that logged
'##### was ist das hier?' and re-raised; it is public API now.
Verified: 213 unit tests pass (from 197); every other suite still collects; the
built wheel contains learning_loop_node/testing/ and no learning_loop_node/
tests/; in a fresh Python 3.10 venv holding only that wheel, the package
imports with pytest absent, and with pytest added a throwaway project uses the
fixtures plugin and the helpers exactly as documented. Repository-wide ruff
goes from 717 findings to 713, with no rule higher than before. This repository
has no pre-commit config and no pylint or pyright in its environment, so ruff
and pytest are what ran.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
klangenk
force-pushed
the
share-generic-node-code
branch
from
August 25, 2026 14:09
08f1e66 to
8e402d6
Compare
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.
Motivation
learning_loop_node/tests/is excluded from the wheel, so nothing in it is importable by a noderepository. That is why the node repositories have almost no tests: each would have to reinvent
TestingTrainerLogic, thedata_folderfixture and theconditionpoller before writing itsfirst assertion.
Separately, a missing
LOOP_HOSTpointed the destructive live-loop fixtures at production.Scope note. This is not part of the node deduplication card — it removes
no lines from any node repository. It was split out of #94 for exactly that reason. What it
deduplicates is internal to this repository; what it adds is the precondition for node repositories
to have offline tests at all.
Implementation
learning_loop_node/testing/, a package that ships in the wheel:helpers.py,detections.py,fixtures.py,trainer.py,detector.pytests/— this library's own suites — excluded from the wheel, as beforepytest_plugins = ['learning_loop_node.testing.fixtures'].Deliberately not a
pytest11entry point:data_folderis autouse and wipes a directory, sono project should get it merely by installing the library. For the same reason
testing/__init__.pydoes not importfixtures, so the package stays importable with pytestabsent
data_folderinto one —mock_trainer's andmock_detector's haddrifted to not create the folder they point at
assert_not_production_loop(), called by every fixture that creates or deletes a project, andmake
run_tests.shfail before pytest starts whenLOOP_HOSTis unset or islearning-loop.ai. CI is unaffected — the workflows pinpreview.learning-loop.aitestingextra declaring pytest; document the package inREADME.mdandAGENTS.mdassert_training_stateloses a debugexcept Exceptionbranch that logged'##### was ist das hier?'and re-raised; it is public API nowVerified
learning_loop_node/testing/and nolearning_loop_node/tests/pytest added, a throwaway project uses the fixtures plugin and the helpers as documented
uv sync --extra dev --frozenstill passes; the lockfile gains the extra with no versionchanges
This repository has no pre-commit config and no pylint or pyright in its environment, so ruff and
pytest are what ran. Committed with
--no-verify.Notes for the reviewer
detector/detections.py+detector/nms.pyandhelpers/subprocess_iterator.py. That divergencecame from Share the generic parts of a node #94, not from here, but it is worth settling before release — this is a published wheel,
so renaming later is a breaking change.
/tmp/learning_loop_lib_datais a fixed, shared path, so two suites must not run concurrently. Aper-run name is the other half of that problem and is not done here.
⬛ claude-opus-5[1m]