Stop the driver sweep at the first driver that claims the image - #44
Open
jasiek wants to merge 1 commit into
Open
Stop the driver sweep at the first driver that claims the image#44jasiek wants to merge 1 commit into
jasiek wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes .img driver detection in the Pyodide runtime by importing CHIRP driver modules incrementally and stopping as soon as the first matching driver class is registered, preserving CHIRP’s import-order–based selection semantics while reducing the number of module fetches in the browser.
Changes:
- Added incremental driver detection in
runtime_bridge.pythat imports modules one-by-one and checks newly registered radios after each import. - Updated
runtime-rpc.jsto use the new incremental detection path and to cache only the “all modules exhausted” state, not partial imports. - Added parity/early-exit and edge-case tests for metadata-less and metadata-unmatched images, plus updated documentation in
FINDINGS.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/python/runtime_bridge.py | Introduces incremental detection helpers and refactors image base64 decode + temp file handling for consistent driver interactions. |
| web/js/runtime-rpc.js | Switches image-load fallback from full driver sweep to incremental detection with early exit and updated session caching semantics. |
| scripts/test-metadataless-image-load.mjs | Adds tests to prove parity with the full sweep, early-exit behavior, pre-registered precedence, and exhausted-list behavior. |
| FINDINGS.md | Updates operational notes and adds an ordering-safety finding documenting why early exit preserves correctness. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Loading an image with no metadata trailer used to import all 191 CHIRP driver modules before detection could run — in the browser that is 191 sequential CDN fetches, the slowest thing the app does. Detection only ever needs the modules up to the winning driver, so import in the same order and stop at the first match instead. Measured over the 108 metadata-less images in chirp/tests/images, the median image is identified after 72.5 of 191 modules and the mean after 78, so roughly 60% of the fetches disappear. Worst case is unchanged. This cannot change which driver is chosen: get_radio_by_image returns the first match in DRV_TO_RADIO insertion order, and insertion order is decided by import order alone. Reordering the list for speed would not be safe, since the default match_model is a bare memory-size comparison several drivers can satisfy at once. Rebased onto the ImageDetectionError backstop from #46: the sweep is now what loadImageWithDriverFallback() injects, so both the unresolved path and the retry after a wrong fast-path resolve stop early. The injection point is renamed importDriversForDetection, since it no longer imports every driver. _image_class_matches() also returns after the metadata-less match_model branch instead of falling into the alias comparison the way upstream does. Upstream compares against meta_vendor/meta_model of None, which no registered class declares, so the two are equivalent — but only the early return makes the docstring's "two branches" true of the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jasiek
force-pushed
the
claude/incremental-driver-loading-e0d2b3
branch
from
August 1, 2026 18:48
a29a0c4 to
119502a
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
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.
What
Loading a
.imgwith no metadata trailer used to import all 191 CHIRP driver modules before detection could run. In the browser every module is its own CDN round trip, so that sweep is the slowest thing the app does. Detection only ever needs the modules up to the winning driver, so the sweep now imports in the same order and stops at the first driver that claims the image.Measured over the 108 metadata-less images in
chirp/tests/images/: all 108 match, min 1 module, p25 30, median 72.5, p75 116, max 188 (Yaesu_VX-8*). Roughly 60% of the fetches disappear at the median; the worst case is unchanged.Why this cannot pick a different driver
directory.get_radio_by_image()returns the first match inDRV_TO_RADIOinsertion order, and insertion order is a pure function of import order. Import the same list in the same order and stop at the first hit, and the winner is exactly the one the full sweep would have returned.What would not be safe is reordering the list to try likely drivers first: 111 registered classes inherit the default
match_model, which is the bare comparisonlen(filedata) == cls._memsize, and several classes share a memory size — among those the winner is decided by order alone. The parity test below is what pins this down.Changes
web/python/runtime_bridge.pydetect_image_driver_incremental(image_b64, module_short_names, progress_cb): imports one module at a time, checks only newly-registered classes after each, returns the match plusimported/total/exhausted._image_class_matches()mirrors the per-class body ofget_radio_by_image— both thematch_modelbranch (metadata-less images) and the vendor/model/variant alias branch (images whose trailer the catalog could not resolve) — including swallowing driver exceptions during detection. One deliberate divergence, from review: it returns after thematch_modelbranch instead of falling into the alias comparison withmeta_vendor/meta_modelbothNonethe way upstream does. No registered class declares aNoneVENDOR or MODEL, so the comparison upstream still runs can never match; the docstring and FINDINGS record that so a future CHIRP bump has something to trip over.DRV_TO_RADIO, and the full sweep would have considered it first too._decode_image_b64and a_temp_image_filecontext manager so detection and loading hand drivers the same.imgfilename shape (some drivers key off the extension).import_all_driver_modulesis untouched —scripts/build-catalog.mjsand the existing tests still use it.web/js/runtime-rpc.jsensureAllDriverModules()→importDriverModulesUntilImageMatches(imageBase64).allDriverModulesImportedboolean set only when the module list is exhausted: "some drivers are imported" is not a reusable fact the way "all of them are" is.load_image_base64→get_radio_by_image, so one code path picks the driver and one reads the image.web/js/image-metadata.mjs— rebased onto theImageDetectionErrorbackstop merged in #46. The incremental sweep is whatloadImageWithDriverFallback()injects, so both paths stop early: the unresolved-image path and the retry after a wrong fast-path resolve. The injected function is renamedimportAllDrivers→importDriversForDetection, because it no longer imports every driver.FINDINGS.md— newincremental-detection-is-order-safeentry with the ordering argument, the measured distribution and the upstream divergence;image-driver-resolutionandall-driver-import-mechanicsupdated to describe the incremental sweep rather than the all-or-nothing one.Tests
Four new cases in
scripts/test-metadataless-image-load.mjs, alongside the retry-gate test from #46:Baofeng_UV-3R, 13th module;Wouxun_KG-UV8D, 94th); a fresh Pyodide runtime per image then runs incremental detection and asserts the same module, class and channel rows, withimported < total. Parity is derived from the full sweep rather than hardcoded, so this is the test that catches a future reordering "optimisation". Also asserts progress reports stop exactly where detection stopped.ensure_radio_module('baofeng_uv3r')first, then detect against a module list that excludes it; matches atimported === 0.exhausted: trueand every module imported.Kenwood_TS-480_CloneMode.imgresolves tots480.TS480_CRadioincrementally (at module 168 of 191), exercising the branch wherematch_modelis never called.npm test— 768/768 pass (177 + 32 + 553 + 6).py_compileonweb/python/runtime_bridge.pyclean.New dependencies
None.
Not in this PR
_ensure_chirp_module_filestill blocks on one fetch at a time, so the remaining ~72 median fetches are strictly serial. Prefetching sources into the Pyodide FS with real concurrency is order-preserving and would compose with this change.🤖 Generated with Claude Code