Skip to content

feat: add SSL+HC fused classifier backend (ssl_fusion) - #91

Open
PouyaMohseni wants to merge 28 commits into
mainfrom
add-ssl-fusion-classifier
Open

feat: add SSL+HC fused classifier backend (ssl_fusion)#91
PouyaMohseni wants to merge 28 commits into
mainfrom
add-ssl-fusion-classifier

Conversation

@PouyaMohseni

@PouyaMohseni PouyaMohseni commented Jul 28, 2026

Copy link
Copy Markdown

Summary

Adds an optional ssl_fusion classifier backend — DINO SimCLR embeddings fused with the existing 29-dim handcrafted (HC) features — as an alternative to the default kNN classifier, selectable per session.

Changes

  • Classifier core: SSLFusionClassifier fuses standardized SSL + HC feature blocks via weighted concatenation, classifies with a linear-kernel SVM.
  • Real-pixel crops: Glyph.image_gray_b64 the SSL extractor needs real colour pixels (matching what the DINO checkpoint was trained on), not the binary mask kNN uses. Colour was previously discarded at ingest and faked back via channel replication — a real train/inference mismatch, now fixed.
  • Backend selection: wired through Session.classify and the /classify API endpoint (backend: "knn" | "ssl_fusion"); new Model selector in the frontend toolbar and upload screen.
  • Precomputed embeddings: companion .ssl_embeddings.npz support for built-in presets (Hufnagel, Square2) and for uploaded GameraXML training files, so ssl_fusion doesn't require a live model pass on every session. match_glyphs_to_source_pages recovers real crops from bbox+mask when only a GameraXML file (no stored crop) is uploaded.
  • Export: /export-embeddings endpoint + UI button, so a session's SSL features can be exported and re-imported as a future preset.
  • Reliability fixes: skip crop-less training glyphs instead of failing the whole fit; fix a calibration crash when a training class has fewer than 5 examples (falls back to an uncalibrated SVC + fixed confidence).
  • Upload UX: file-chip remove buttons on the upload screen, plus a stale-closure fix that broke re-picking a removed file.

Summary by CodeRabbit

  • New Features

    • Added optional SSL-fusion classification using pre-trained image features and handcrafted features.
    • Added model selection between HC + kNN and Pre-trained + SVM.
    • Added support for uploading training embeddings and source images.
    • Added SSL-compatible preset indicators and companion embeddings export.
    • Sessions remain editable and reusable after export.
  • Bug Fixes

    • Improved classification error handling so edits and refreshes complete reliably.
    • Added validation for missing, mismatched, or unusable embedding data.

…scale

The DINO SimCLR checkpoint was trained on real colour manuscript
photographs, but the ingest pipeline converted pages to greyscale
before slicing the "real-pixel" crop, then faked RGB by channel
replication at extraction time -- a genuine train/inference domain
mismatch. Store and slice colour crops instead, and regenerate the
Hufnagel/Square2 preset embeddings to match.
…t=4.0

Re-validated on the corrected colour-crop pipeline across
leave-one-manuscript-out (Hufnagel/MS234/Antiphonal+NZ) and a
single-manuscript stress test: the previous RBF kernel collapses onto
majority classes whenever trained on one small/imbalanced manuscript
(the common case -- a user selecting a single training preset). This
linear config beat every alternative tested, including being the
first to beat the handcrafted-feature kNN baseline on the
single-manuscript stress test (79.4% vs 77.0%).
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f809d0b5-82a9-4627-bef2-9e193cdb90de

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Adds optional SSL-fusion classification with cached glyph features, companion embedding ingestion/export, API contracts, and frontend controls. Session classification remains backend-selectable, while exports remain read-only and sessions stay editable after completion.

Changes

SSL fusion workflow

Layer / File(s) Summary
Feature storage and preset matching
core/ic_core/src/ic_core/{glyph,image,ingest,ssl_preset_embeddings}.py, core/tests/test_ssl_preset_embeddings.py
Glyphs and ingested pages support optional image crops and SSL embeddings; preset loading, validation, attachment, and source-page matching are covered by tests.
SSL extractor and classifier
core/ic_core/src/ic_core/{classifier,state,ssl_classifier,ssl_extractor}.py, core/scripts/*ssl_embeddings.py, core/tests/test_ssl_{classifier,extractor}.py, core/ic_core/pyproject.toml
Adds the optional SSL extractor, fused classifier, backend factory, classifier injection, checkpoint handling, generation scripts, and optional dependencies.
API training and export workflow
api/src/ic_api/{main,schemas}.py, api/tests/test_api.py, api/pyproject.toml
Training uploads accept companion embeddings or images, presets expose compatibility metadata, classification accepts backend selection, and a read-only embeddings export endpoint is added.
Frontend backend and export controls
frontend/src/{api,components,constants,hooks,store,types}/...
Adds backend selection, companion-file uploads, backend-aware classification, post-create reclassification, model status, and embeddings export controls.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant UploadView
  participant API
  participant Session
  participant SSLFusionClassifier
  participant ViTExtractor
  UploadView->>API: upload training XML and companion assets
  API->>Session: create session with enriched glyphs
  UploadView->>API: classify with ssl_fusion
  API->>Session: select classification backend
  Session->>SSLFusionClassifier: fit and predict glyphs
  SSLFusionClassifier->>ViTExtractor: extract missing SSL features
  ViTExtractor-->>SSLFusionClassifier: return embeddings
  SSLFusionClassifier-->>Session: return classifications
  Session-->>API: return updated session
  API-->>UploadView: return classification result
Loading

Suggested reviewers: yueqiao12zhang

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 51.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly captures the main change: adding the optional ssl_fusion fused classifier backend.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch add-ssl-fusion-classifier

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/EditPanel.tsx (1)

103-108: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not make successful label updates depend on reclassification succeeding.

The edit mutations can persist changes before the selected classifier runs. If classification fails, cache refresh and related bookkeeping are skipped, leaving the UI stale despite successful server updates.

  • frontend/src/components/EditPanel.tsx#L103-L108: record the edit and refresh/retain undo state independently of classification success.
  • frontend/src/hooks/useUpdateGlyphs.ts#L76-L80: invalidate the session after per-glyph updates even when classification rejects.
  • frontend/src/hooks/useUpdateGlyphs.ts#L107-L111: invalidate the session after bulk updates even when classification rejects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/EditPanel.tsx` around lines 103 - 108, Make label
persistence and session invalidation independent of classifier success: in
frontend/src/components/EditPanel.tsx lines 103-108, ensure the edit and
undo-state bookkeeping are retained and the session cache is refreshed even when
classify.mutateAsync rejects; in frontend/src/hooks/useUpdateGlyphs.ts lines
76-80 and 107-111, ensure both per-glyph and bulk update flows invalidate the
session after updates regardless of classification failure.
🧹 Nitpick comments (8)
core/data/presets/Square2.ssl_embeddings.npz (1)

1-1: 🧹 Nitpick | 🔵 Trivial

Binary asset — verify regeneration/versioning strategy.

This is a generated binary artifact (precomputed SSL embeddings). Since it's derived data tied to a specific SSL extractor/model version, consider documenting how it's regenerated (e.g., via generate_square2_ssl_embeddings.py) and whether large binary blobs like this should be tracked via Git LFS instead of committed directly to the repo, to avoid repo bloat as embeddings get regenerated over time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/data/presets/Square2.ssl_embeddings.npz` at line 1, Document the
regeneration and versioning strategy for the generated Square2 SSL embeddings,
referencing generate_square2_ssl_embeddings.py and the extractor/model version
used. Clarify whether this binary should remain in Git or be managed through Git
LFS, and update repository tracking accordingly if the project’s policy requires
it.
frontend/src/components/UploadView.tsx (1)

168-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared training-args computation from handleSubmit/handleAutoExport.

training/embeddings/images/presetNames are recomputed identically across 4 call sites (2 in handleSubmit, 2 in handleAutoExport). A small helper (or useMemo) would remove the duplication and keep the two flows in sync as new training-data fields get added later.

♻️ Suggested extraction
+  function buildTrainingArgs() {
+    return {
+      trainingFiles: trainingFiles.length > 0 ? trainingFiles : undefined,
+      trainingEmbeddings:
+        trainingEmbeddings.length > 0 ? trainingEmbeddings : undefined,
+      trainingImages: trainingImages.length > 0 ? trainingImages : undefined,
+      trainingPresets:
+        selectedPresets.length > 0 ? selectedPresets : undefined,
+    };
+  }

Then spread ...buildTrainingArgs() into each mutate(...) call.

Also applies to: 204-239

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/UploadView.tsx` around lines 168 - 198, Extract the
shared training-data argument construction from handleSubmit and
handleAutoExport into a reusable helper, such as buildTrainingArgs, using
trainingFiles, trainingEmbeddings, trainingImages, and selectedPresets with
their existing empty-to-undefined behavior. Spread the helper result into all
four createFromStaging.mutate and create.mutate calls, preserving the existing
non-training fields and keeping both submission flows synchronized.
core/ic_core/src/ic_core/classifier.py (1)

500-501: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a Protocol instead of object for the classifier-factory contract.

Widening the return type to object (from InteractiveClassifier) is a reasonable trade-off to support pluggable backends, but it means any caller that keeps the returned classifier to reuse .fit()/.predict_many() loses static-type checking entirely — the docstring even states the contract ("a zero-argument callable returning an unfitted classifier with .fit(), .predict_many()") but nothing enforces it for a type checker. A small Protocol captures this structurally without requiring InteractiveClassifier/SSLFusionClassifier to share a common base class.

♻️ Suggested Protocol
-from typing import Callable, Iterable, Sequence
+from typing import Callable, Iterable, Protocol, Sequence
+
+
+class TrainableClassifier(Protocol):
+    def fit(self, training_glyphs: Sequence[Glyph]) -> "TrainableClassifier": ...
+    def predict_many(self, glyphs: Sequence[Glyph]) -> list[Prediction]: ...
 def run_correction_stage(
     glyphs: Sequence[Glyph],
     training_glyphs: Sequence[Glyph] | None = None,
     *,
     k: int = DEFAULT_K,
-    classifier_factory: Callable[[], object] | None = None,
-) -> tuple[list[Glyph], object]:
+    classifier_factory: Callable[[], TrainableClassifier] | None = None,
+) -> tuple[list[Glyph], TrainableClassifier]:

Also applies to: 536-550

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/ic_core/src/ic_core/classifier.py` around lines 500 - 501, Define a
classifier protocol exposing the required unfitted-classifier methods, including
fit() and predict_many(), and use it for the classifier_factory return type and
the enclosing function’s returned classifier type instead of object. Ensure
InteractiveClassifier and SSLFusionClassifier satisfy the protocol structurally
so callers retaining the classifier preserve static type checking.
core/tests/test_ssl_classifier.py (1)

173-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Asserting on the private _calibrated flag couples the tests to internals.

Consider exposing a small read-only property (e.g. is_calibrated) on SSLFusionClassifier and asserting on that instead; behaviourally the confidence assertions already carry most of the signal.

Also applies to: 199-199

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/tests/test_ssl_classifier.py` around lines 173 - 176, Expose a read-only
public calibration-status property such as is_calibrated on SSLFusionClassifier,
backed by the existing calibration state, and update the affected tests to
assert through that property instead of accessing the private _calibrated flag.
Keep the existing confidence and class-name assertions unchanged.
core/ic_core/src/ic_core/ssl_classifier.py (1)

157-166: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Mixed precomputed/live batches assume identical dimensionality.

X_ssl is allocated from len(precomputed[0].ssl_embedding) and then assigned the live extractor output; if the configured checkpoint's cls_mean width differs from the shipped preset embeddings (e.g. a different backbone), this fails with an opaque numpy broadcast error rather than a message pointing at the checkpoint/preset mismatch. A short explicit width check would be worth it here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/ic_core/src/ic_core/ssl_classifier.py` around lines 157 - 166, In the
mixed precomputed/live branch, validate that the live output from _ssl_features
has the same embedding width as the precomputed ssl_embedding width before
assigning into X_ssl. Raise a clear error identifying the
checkpoint-versus-preset dimensionality mismatch instead of allowing a NumPy
broadcast failure.
api/src/ic_api/main.py (1)

1511-1517: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Full model pass runs while holding the per-session lock.

extract_ssl_embeddings can load a ViT and run a batched forward pass over every selected glyph; doing it inside with store.session(...) serialises every other request for that session behind it (classify, glyph updates) for potentially minutes. Snapshotting the selected glyphs under the lock and extracting outside it would keep the session responsive.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/src/ic_api/main.py` around lines 1511 - 1517, Update the flow around the
store.session block in the export handler so _select_export_glyphs snapshots the
required glyph data while the per-session lock is held, then exit the context
before calling extract_ssl_embeddings. Preserve the existing selected-glyph
inputs and output serialization, but ensure the model load and batched inference
cannot hold the session lock.
core/scripts/generate_hufnagel_ssl_embeddings.py (2)

57-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse match_glyphs_to_source_pages instead of the hand-rolled RLE/matching duplicate.

_decode_rle_mask, _parse_preset_glyphs (regex over XML) and the per-glyph brute-force loop reimplement what ic_core.io_xml.load_glyphs + ic_core.ssl_preset_embeddings.match_glyphs_to_source_pages already do — exactly the shape the Square2 script uses. Keeping two matching implementations risks the two presets' embeddings drifting apart (threshold, greyscale conversion, glyph ordering).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/scripts/generate_hufnagel_ssl_embeddings.py` around lines 57 - 99,
Replace the local _decode_rle_mask, _parse_preset_glyphs, and per-glyph
page-matching loop in main with ic_core.io_xml.load_glyphs and
ic_core.ssl_preset_embeddings.match_glyphs_to_source_pages, following the
existing Square2 script usage. Preserve the resulting glyph ordering, matched
crops, and unmatched handling while using the shared implementation for
thresholding and grayscale conversion; remove the now-unused duplicate helpers
and related logic.

42-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hard-coded absolute paths from a developer's machine.

SOURCE_PAGES points at /Users/home/Desktop/DDMAL/..., so this script only runs on one laptop even though the sibling core/scripts/generate_square2_ssl_embeddings.py resolves its page relative to the repo (and the pages appear to live under core/data/train/). Suggest repo-relative paths with an env-var override.

♻️ Proposed change
-SOURCE_PAGES = [
-    Path(
-        "/Users/home/Desktop/DDMAL/standalone-interactive-classifier/core/data/train/hufnagel_example_826dd1b4.png"
-    ),
-    Path(
-        "/Users/home/Desktop/DDMAL/standalone-interactive-classifier/core/data/train/hufnagel_example_a77ec16f.png"
-    ),
-    Path(
-        "/Users/home/Desktop/DDMAL/standalone-interactive-classifier/core/data/train/hufnagel_example_fbed8126.png"
-    ),
-]
+_TRAIN_DIR = Path(__file__).resolve().parents[1] / "data" / "train"
+SOURCE_PAGES = [
+    _TRAIN_DIR / "hufnagel_example_826dd1b4.png",
+    _TRAIN_DIR / "hufnagel_example_a77ec16f.png",
+    _TRAIN_DIR / "hufnagel_example_fbed8126.png",
+]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/scripts/generate_hufnagel_ssl_embeddings.py` around lines 42 - 52,
Update SOURCE_PAGES in generate_hufnagel_ssl_embeddings.py to resolve the
training page paths relative to the repository or script location instead of
using developer-specific absolute paths, while supporting an
environment-variable override for the data root. Preserve the existing three
page filenames and align the resolution approach with
generate_square2_ssl_embeddings.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/ic_core/src/ic_core/ssl_classifier.py`:
- Around line 265-293: The predict_many method must handle glyphs lacking both
ssl_embedding and image_gray_b64 before calling _fused_features, matching
fit()’s filtering behavior. Filter or otherwise validate these crop-less glyphs
up front so one invalid glyph cannot cause an unhandled crop_glyph_to_square
failure; if rejecting them, raise an actionable error identifying the affected
glyphs.

In `@core/ic_core/src/ic_core/ssl_extractor.py`:
- Around line 161-162: Update the checkpoint loading call in the SSL extractor
to pass weights_only=True to torch.load while preserving the existing CPU map
location and subsequent backbone.load_state_dict(state) flow.
- Around line 298-337: Update _apply_adapters and _apply_ssf to traverse
ViTModel’s encoder.layer modules instead of backbone.layers, targeting
attention.output.dense, intermediate.dense, and output.dense. Preserve the
existing adapter hook behavior and SSF wrapping while ensuring method.json
checkpoint loading can inject modules before load_state_dict.

In `@core/ic_core/src/ic_core/ssl_preset_embeddings.py`:
- Around line 85-93: Update the glyph/embedding validation before the list
comprehension in the preset embedding loader to require a 2-D floating embedding
matrix with finite values, raising ValueError for invalid shape, dtype, or
non-finite entries. Preserve the existing count-mismatch validation and only
attach embeddings after all validation succeeds.

In `@core/ic_core/src/ic_core/state.py`:
- Around line 281-288: Update the `backend` parameter docstring to describe
`ssl_fusion` as using the SSL+HC fused linear-kernel SVM classifier, calibrated
via `CalibratedClassifierCV`, instead of calling it a logistic-regression
classifier; preserve the existing requirements and references.

In `@core/tests/test_ssl_classifier.py`:
- Around line 169-171: Update the classifier factories in the affected tests,
including test_fits_with_few_examples_per_class_shrinks_cv, to construct
SSLFusionClassifier with _SSL_CHECKPOINT instead of leaving checkpoint unset.
Ensure both tests use a factory that forwards this checkpoint so they bypass
network downloads while preserving their existing test behavior.

In `@frontend/src/constants/classifierBackends.ts`:
- Around line 16-24: Update the SSL-fusion classifier entry’s user-facing label
and title to identify linear-kernel SVM rather than logistic regression, and
revise the requirements text to state that uploaded GameraXML files are
supported when companion embeddings or source images are available. Keep the
remaining SSL checkpoint, preset, and server-extra requirements accurate.

In `@frontend/src/hooks/useCreateSession.ts`:
- Around line 25-36: Handle rejection from the fire-and-forget classify call in
reclassifyIfSslFusionChosen by adding explicit failure handling that surfaces
the error instead of allowing an unhandled promise rejection. Keep successful
query invalidation unchanged.

---

Outside diff comments:
In `@frontend/src/components/EditPanel.tsx`:
- Around line 103-108: Make label persistence and session invalidation
independent of classifier success: in frontend/src/components/EditPanel.tsx
lines 103-108, ensure the edit and undo-state bookkeeping are retained and the
session cache is refreshed even when classify.mutateAsync rejects; in
frontend/src/hooks/useUpdateGlyphs.ts lines 76-80 and 107-111, ensure both
per-glyph and bulk update flows invalidate the session after updates regardless
of classification failure.

---

Nitpick comments:
In `@api/src/ic_api/main.py`:
- Around line 1511-1517: Update the flow around the store.session block in the
export handler so _select_export_glyphs snapshots the required glyph data while
the per-session lock is held, then exit the context before calling
extract_ssl_embeddings. Preserve the existing selected-glyph inputs and output
serialization, but ensure the model load and batched inference cannot hold the
session lock.

In `@core/data/presets/Square2.ssl_embeddings.npz`:
- Line 1: Document the regeneration and versioning strategy for the generated
Square2 SSL embeddings, referencing generate_square2_ssl_embeddings.py and the
extractor/model version used. Clarify whether this binary should remain in Git
or be managed through Git LFS, and update repository tracking accordingly if the
project’s policy requires it.

In `@core/ic_core/src/ic_core/classifier.py`:
- Around line 500-501: Define a classifier protocol exposing the required
unfitted-classifier methods, including fit() and predict_many(), and use it for
the classifier_factory return type and the enclosing function’s returned
classifier type instead of object. Ensure InteractiveClassifier and
SSLFusionClassifier satisfy the protocol structurally so callers retaining the
classifier preserve static type checking.

In `@core/ic_core/src/ic_core/ssl_classifier.py`:
- Around line 157-166: In the mixed precomputed/live branch, validate that the
live output from _ssl_features has the same embedding width as the precomputed
ssl_embedding width before assigning into X_ssl. Raise a clear error identifying
the checkpoint-versus-preset dimensionality mismatch instead of allowing a NumPy
broadcast failure.

In `@core/scripts/generate_hufnagel_ssl_embeddings.py`:
- Around line 57-99: Replace the local _decode_rle_mask, _parse_preset_glyphs,
and per-glyph page-matching loop in main with ic_core.io_xml.load_glyphs and
ic_core.ssl_preset_embeddings.match_glyphs_to_source_pages, following the
existing Square2 script usage. Preserve the resulting glyph ordering, matched
crops, and unmatched handling while using the shared implementation for
thresholding and grayscale conversion; remove the now-unused duplicate helpers
and related logic.
- Around line 42-52: Update SOURCE_PAGES in generate_hufnagel_ssl_embeddings.py
to resolve the training page paths relative to the repository or script location
instead of using developer-specific absolute paths, while supporting an
environment-variable override for the data root. Preserve the existing three
page filenames and align the resolution approach with
generate_square2_ssl_embeddings.py.

In `@core/tests/test_ssl_classifier.py`:
- Around line 173-176: Expose a read-only public calibration-status property
such as is_calibrated on SSLFusionClassifier, backed by the existing calibration
state, and update the affected tests to assert through that property instead of
accessing the private _calibrated flag. Keep the existing confidence and
class-name assertions unchanged.

In `@frontend/src/components/UploadView.tsx`:
- Around line 168-198: Extract the shared training-data argument construction
from handleSubmit and handleAutoExport into a reusable helper, such as
buildTrainingArgs, using trainingFiles, trainingEmbeddings, trainingImages, and
selectedPresets with their existing empty-to-undefined behavior. Spread the
helper result into all four createFromStaging.mutate and create.mutate calls,
preserving the existing non-training fields and keeping both submission flows
synchronized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 479fda2e-9e10-4e07-815a-e21502b04f4b

📥 Commits

Reviewing files that changed from the base of the PR and between debe39d and f35d104.

⛔ Files ignored due to path filters (3)
  • api/uv.lock is excluded by !**/*.lock
  • core/data/train/Einsiedeln__Stiftsbibliothek__Codex_611_014r.jpg is excluded by !**/*.jpg
  • core/ic_core/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (32)
  • api/pyproject.toml
  • api/src/ic_api/main.py
  • api/src/ic_api/schemas.py
  • api/tests/test_api.py
  • core/data/presets/Hufnagel.ssl_embeddings.npz
  • core/data/presets/Square2.ssl_embeddings.npz
  • core/ic_core/pyproject.toml
  • core/ic_core/src/ic_core/classifier.py
  • core/ic_core/src/ic_core/glyph.py
  • core/ic_core/src/ic_core/image.py
  • core/ic_core/src/ic_core/ingest.py
  • core/ic_core/src/ic_core/ssl_classifier.py
  • core/ic_core/src/ic_core/ssl_extractor.py
  • core/ic_core/src/ic_core/ssl_preset_embeddings.py
  • core/ic_core/src/ic_core/state.py
  • core/scripts/generate_hufnagel_ssl_embeddings.py
  • core/scripts/generate_square2_ssl_embeddings.py
  • core/tests/test_ssl_classifier.py
  • core/tests/test_ssl_preset_embeddings.py
  • frontend/src/api/sessions.ts
  • frontend/src/components/EditPanel.tsx
  • frontend/src/components/SplitDialog.tsx
  • frontend/src/components/Toolbar.tsx
  • frontend/src/components/UploadView.tsx
  • frontend/src/constants/classifierBackends.ts
  • frontend/src/hooks/useAutoExport.ts
  • frontend/src/hooks/useClassify.ts
  • frontend/src/hooks/useCreateSession.ts
  • frontend/src/hooks/useExportEmbeddings.ts
  • frontend/src/hooks/useUpdateGlyphs.ts
  • frontend/src/store/uiStore.ts
  • frontend/src/types/api.ts
👮 Files not reviewed due to content moderation or server errors (1)
  • core/data/presets/Hufnagel.ssl_embeddings.npz

Comment thread core/ic_core/src/ic_core/ssl_classifier.py
Comment thread core/ic_core/src/ic_core/ssl_extractor.py Outdated
Comment thread core/ic_core/src/ic_core/ssl_extractor.py
Comment thread core/ic_core/src/ic_core/ssl_preset_embeddings.py
Comment thread core/ic_core/src/ic_core/state.py Outdated
Comment thread core/tests/test_ssl_classifier.py
Comment thread frontend/src/constants/classifierBackends.ts Outdated
Comment thread frontend/src/hooks/useCreateSession.ts
@PouyaMohseni

Copy link
Copy Markdown
Author

@coderabbitai, review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

@PouyaMohseni, I’ll review the changes in #91.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Yueqiao12Zhang

Yueqiao12Zhang commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

By examine the result manually, I found a few UI issue and have a few comments about the performance.

UI issues:

  • I'm not seeing the text for preset training data, I only see the checkboxes. What are those?
  • Is this branch for the mothra embedded version or the stand-alone version?
  • In the training data viz menu, are all training data there been used?

Comments:

  • Apparently the features are more robust.
  • SVM are less sensitive to changes and re-classification.

Please view the changes of this PR in the stand-alone version and the mothra embedded version separately to see the compatibility. Merge to develop branch and main branch separately, as they may contain different environments for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants