[drewoldag]

Change Description

Every @hyrax_model class was constructed as Model(config=config, data_sample=prepared_sample), and feeding that sample required a pre-flight layer: DataProvider.sample_data, StreamingDataProvider.sample_data, HyraxCSVDataset.sample_data, the Kafka and LSDB peek_sample methods with replay buffers and a peek_timeout key, and setup_model_from_sample for infer_stream. Only five of nine built-in models read the sample, one of those could already get the value from config, and SimCLR could not be constructed through setup_model at all.

This PR removes data_sample outright, with no compatibility shim, and deletes the pre-flight machinery with it.

Solution Description

Model contract. Hyrax constructs a model as Model(config) and passes nothing else. setup_model(config) takes no dataset, and setup_model_from_sample and reset_trace (which only existed to clean up after the pre-flight) are gone.

Image models read [model.<Name>].input_shape. HyraxCNN, HyraxAutoencoder, HyraxAutoencoderV2, and ImageDCAE size their layers from a [channels, height, width] list, defaulting to [3, 32, 32] to match HyraxCifarDataset. A shared read_input_shape helper in model_utils validates it (three positive ints, false rejected) and raises a ValueError naming the key. Users of other datasets set it alongside [data_set].crop_to. While here, the swapped width/height assignment in the autoencoders and CNN is fixed, which only changes behavior for non-square inputs.

Other models. HyraxTs2Vec sizes its input from EncodingSettings(config).n_channels, which it already computed. HSCAutoencoder, HSCDCAE, HyraxLoopback, and SimCLR just drop the parameter.

infer_stream. run() no longer takes sample_batch. A configured [data_request.infer_stream] group yields an iterable session; otherwise a manual session that you feed through session.process(batch).

Peek machinery deleted. Kafka loses peek_sample, PEEK_WARN_INTERVAL, peek_timeout, and _buffered. LSDB loses peek_sample and _peek_index but keeps _buffered, which __iter__ still uses to push back undelivered rows. Migration 007 drops peek_timeout from existing configs.

Docs. model_class_reference.rst, HYRAX_GUIDE.md, and the maintained notebooks follow the new contract. external_model_class.ipynb and lsdb_stream_dataset.ipynb now read shapes from a config entry or a module constant. Archived notebooks are left as history.

Testing

• python -m pytest tests/ -m "not slow": 810 passed.
• python -m pytest tests/hyrax/test_e2e.py: 6 passed. The hsc1k demo data has three filters and cutouts of at least 96x96, so HSC and FITS run with crop_to = [96, 96] and input_shape = [3, 96, 96]; CIFAR runs on the default.
• New tests/hyrax/test_input_shape.py checks that each image model builds from the shipped default and that malformed input_shape values (wrong length, non-int, zero, negative, bool, false, scalar) raise a ValueError naming the key.
• Peek tests removed; manual-mode infer_stream tests call h.infer_stream() with no argument.
• pre-commit run --all-files passes with the sphinx hook skipped.

Code Quality

• I have read the Contribution Guide and agree to the Code of Conduct
• My code follows the code style of this project
• My code builds (or compiles) cleanly without any errors or warnings
• My code contains relevant comments and necessary documentation

🤖 Generated with Claude Code

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[codecov]

Codecov Report

❌ Patch coverage is 98.27586% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.56%. Comparing base (dade46f) to head (4a80b47).

Files with missing lines	Patch %	Lines
src/hyrax/models/hyrax_autoencoder.py	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1082      +/-   ##
==========================================
+ Coverage   71.02%   71.56%   +0.53%     
==========================================
  Files          92       93       +1     
  Lines        9381     9309      -72     
==========================================
- Hits         6663     6662       -1     
+ Misses       2718     2647      -71     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[drewoldag]

This is looking pretty correct to me. One small update to the way a config value is being accessed, and a comment about docstring in infer_stream (not a blocker)

[drewoldag]

This seems odd to me. The original reasoning for having this functionality isn't clear at this point either. I don't think there's any code to remove. I think just removing the documentation would suffice.

[mtauraso]

LGTM