Fix: broken DST fallback path and dead test_files/test_dst_generator.py (found in #63 research) #74

New issue

Closed

opened 2026-06-23 20:14:13 +00:00 by Mark · 1 comment

Mark commented

2026-06-23 20:14:13 +00:00

Owner

Problem

During the research phase of #63 (SVG → DST round trip), kilo surfaced two latent issues in the embroidery pipeline that are out of scope for #63 but worth fixing:

1. Broken DST fallback in `src/output_generators/dst_generator.py`

The _generate_fallback_dst() method was introduced in commit 0a0a3b4 (the refactor that replaced the working triplet encoder with a pyembroidery-first path + hand-written fallback). The fallback was never validated against the DST spec or against pyembroidery round-trip. It produces invalid output:

Does not start with the correct DST magic (LA:)
Stitch data is corrupt
Only "works" because pyembroidery is normally installed, so the broken fallback is silent until exercised

The refactor commit 0a0a3b4 also removed the functions encode_command_triplet, encode_coordinate_triplet, and generate_dst_header from dst_generator.py.

2. Stale tests in `test_files/test_dst_generator.py`

The file test_files/test_dst_generator.py still imports the removed functions:

encode_command_triplet
encode_coordinate_triplet
generate_dst_header

These imports will fail with ImportError if anyone tries to run them. CI runs only tests/ (not test_files/), so the stale file has been silently dead since the refactor.

What's needed

Either fix _generate_fallback_dst() to produce valid DST bytes (with header, stitch data, END record), OR remove the fallback and require pyembroidery as a hard dependency with a clear error at import time.
Update or delete test_files/test_dst_generator.py. If kept, it should test the current generator class API (not the removed standalone functions). If deleted, also delete the test_files/ directory if it has no other contents.
Add a CI check that fails if any file under test_files/ is referenced by the CI workflow (so stale tests can't silently rot).

Acceptance

process(input_bytes=..., output_format="dst") produces valid DST bytes whether or not pyembroidery is installed. (If pyembroidery is missing, fail with a clear ImportError, not a corrupt fallback.)
No file under test_files/ imports functions that don't exist.
CI surfaces test_files/ failures if anyone runs them.

Decomposed from easystitch#63

## Problem During the research phase of #63 (SVG → DST round trip), kilo surfaced two latent issues in the embroidery pipeline that are out of scope for #63 but worth fixing: ### 1. Broken DST fallback in `src/output_generators/dst_generator.py` The `_generate_fallback_dst()` method was introduced in commit `0a0a3b4` (the refactor that replaced the working triplet encoder with a `pyembroidery`-first path + hand-written fallback). The fallback was never validated against the DST spec or against pyembroidery round-trip. It produces invalid output: - Does not start with the correct DST magic (`LA:`) - Stitch data is corrupt - Only "works" because `pyembroidery` is normally installed, so the broken fallback is silent until exercised The refactor commit `0a0a3b4` also removed the functions `encode_command_triplet`, `encode_coordinate_triplet`, and `generate_dst_header` from `dst_generator.py`. ### 2. Stale tests in `test_files/test_dst_generator.py` The file `test_files/test_dst_generator.py` still imports the removed functions: - `encode_command_triplet` - `encode_coordinate_triplet` - `generate_dst_header` These imports will fail with `ImportError` if anyone tries to run them. CI runs only `tests/` (not `test_files/`), so the stale file has been silently dead since the refactor. ## What's needed - Either fix `_generate_fallback_dst()` to produce valid DST bytes (with header, stitch data, END record), OR remove the fallback and require `pyembroidery` as a hard dependency with a clear error at import time. - Update or delete `test_files/test_dst_generator.py`. If kept, it should test the *current* generator class API (not the removed standalone functions). If deleted, also delete the `test_files/` directory if it has no other contents. - Add a CI check that fails if any file under `test_files/` is referenced by the CI workflow (so stale tests can't silently rot). ## Acceptance - `process(input_bytes=..., output_format="dst")` produces valid DST bytes whether or not `pyembroidery` is installed. (If pyembroidery is missing, fail with a clear ImportError, not a corrupt fallback.) - No file under `test_files/` imports functions that don't exist. - CI surfaces `test_files/` failures if anyone runs them. _Decomposed from [easystitch#63](https://forgejo.tail018ac4.ts.net/Mark/easystitch/issues/63)_

Mark added the

bug

triage-approved

needs-decompose

labels

2026-06-23 20:14:13 +00:00

Mark commented

2026-06-23 20:15:59 +00:00

Author

Owner

Auto-triage: needs decompose

Marked triage-approved + needs-decompose. decompose-executor will pick this up and break it into implementable sub-issues.

## Auto-triage: needs decompose Marked `triage-approved` + `needs-decompose`. decompose-executor will pick this up and break it into implementable sub-issues.

Mark

2026-06-25 18:20:57 +00:00