docs(report): add implementation notes for DICOMweb proxy recommendation
This commit is contained in:
125
report/01-dicom-web-proxy-recommendation.md
Normal file
125
report/01-dicom-web-proxy-recommendation.md
Normal file
@@ -0,0 +1,125 @@
|
|||||||
|
# Report: DICOMweb proxy recommendation
|
||||||
|
|
||||||
|
- Todo: `todo/01-dicom-web-proxy-recommendation.md`
|
||||||
|
- Detail: `todo/01-dicom-web-proxy-recommendation.detail.md`
|
||||||
|
- Branch: `feature/dicomweb-proxy-recommendation`
|
||||||
|
- Commits: `0c680b6`, `3ac04f9`
|
||||||
|
- Status: implemented, build + lint + tests pass
|
||||||
|
|
||||||
|
## What was implemented
|
||||||
|
|
||||||
|
### P0 - metadata display correctness
|
||||||
|
|
||||||
|
The hand-built metadata subset in `parseMeta.ts` was replaced with a
|
||||||
|
VR-aware DICOM-to-DICOM-JSON converter (`src/dimse/dicomJson.ts`).
|
||||||
|
|
||||||
|
| Requirement | Result |
|
||||||
|
| :------------------------------------------- | :----- |
|
||||||
|
| Stop fabricating Window Center (0028,1050) | Done |
|
||||||
|
| Stop fabricating Window Width (0028,1051) | Done |
|
||||||
|
| Stop fabricating Rescale Intercept (0028,1052)| Done |
|
||||||
|
| Stop fabricating Rescale Slope (0028,1053) | Done |
|
||||||
|
| Preserve multi-valued WC/WW and rescale | Done |
|
||||||
|
| Preserve VOI/LUT/presentation/padding tags | Done |
|
||||||
|
| Omit absent optional tags (no `[undefined]`) | Done |
|
||||||
|
|
||||||
|
Converter behavior:
|
||||||
|
|
||||||
|
- Uses explicit `element.vr`, then the DICOM dictionary as fallback.
|
||||||
|
- Resolves ambiguous dictionary VRs (for example `US or SS` for padding).
|
||||||
|
- Handles string, numeric, `DS`/`IS`, `AT`, `PN` and binary VRs.
|
||||||
|
- Recursively converts `SQ` items and keeps LUT sequences.
|
||||||
|
- Encodes binary LUT payloads as `InlineBinary` (base64).
|
||||||
|
- Excludes Pixel Data, float pixel data and overlay data.
|
||||||
|
- Excludes private (odd-group) tags by default.
|
||||||
|
|
||||||
|
### P1 - WADO-RS frame correctness
|
||||||
|
|
||||||
|
| Requirement | Result |
|
||||||
|
| :-------------------------------------------- | :----- |
|
||||||
|
| Validate frame as positive 1-based integer | Done |
|
||||||
|
| Return 400 for invalid values | Done |
|
||||||
|
| Thread frame into `doWadoRs`/buffer builder | Done |
|
||||||
|
| Return only requested uncompressed frame | Done |
|
||||||
|
| Explicit unsupported response for compressed | Done (415) |
|
||||||
|
| Never fall back to full Pixel Data | Done |
|
||||||
|
| JPEG only on `/rendered` and `/thumbnail` | Done |
|
||||||
|
| Frame log with `jpegRendererUsed=false` | Done |
|
||||||
|
|
||||||
|
Frame slicing details:
|
||||||
|
|
||||||
|
- `NumberOfFrames` (0028,0008) defaults to 1 only when absent.
|
||||||
|
- Validates `1 <= frame <= NumberOfFrames`.
|
||||||
|
- Frame size uses Rows, Columns, SamplesPerPixel and BitsAllocated.
|
||||||
|
- Supports 1-bit and byte-aligned BitsAllocated.
|
||||||
|
- Too-short Pixel Data is a 500 server error, not a full-buffer fallback.
|
||||||
|
- Compressed or encapsulated Pixel Data is rejected with 415.
|
||||||
|
- Frame `Content-Location` uses the canonical `/instances/.../frames/{n}` URL.
|
||||||
|
|
||||||
|
### P2 - tests and verification
|
||||||
|
|
||||||
|
| Requirement | Result |
|
||||||
|
| :------------------------------------------- | :----- |
|
||||||
|
| Synthetic non-PHI CR and multi-frame fixtures| Done |
|
||||||
|
| Absent WC/WW stays absent | Done |
|
||||||
|
| Absent rescale stays absent | Done |
|
||||||
|
| Multi-valued WC/WW preserved | Done |
|
||||||
|
| `/frames/1` and `/frames/2` differ | Done |
|
||||||
|
| Invalid frame numbers return 400 | Done |
|
||||||
|
| Compressed extraction returns 415/501 | Done |
|
||||||
|
| `npm run build` and test suite | Done |
|
||||||
|
|
||||||
|
## Files changed
|
||||||
|
|
||||||
|
| File | Change |
|
||||||
|
| :------------------------------ | :----- |
|
||||||
|
| `src/dimse/dicomJson.ts` | New DICOM-to-DICOM-JSON converter |
|
||||||
|
| `src/dimse/frameExtractor.ts` | New native frame slicing + param parser |
|
||||||
|
| `src/dimse/errors.ts` | New status-bearing error types |
|
||||||
|
| `src/dimse/parseMeta.ts` | Uses the converter instead of a static map |
|
||||||
|
| `src/dimse/wadoRs.ts` | Threads frame, logs, frame Content-Location |
|
||||||
|
| `src/routes/routes.ts` | Validates frame, maps error status codes |
|
||||||
|
| `tests/dicomJson.test.ts` | Metadata regression tests |
|
||||||
|
| `tests/frameExtractor.test.ts` | Frame extraction tests |
|
||||||
|
| `tests/helpers/dicomWriter.ts` | Synthetic DICOM writer (test-only) |
|
||||||
|
| `package.json` | Added `npm test` script |
|
||||||
|
| `tsconfig.json` | Added `skipLibCheck` |
|
||||||
|
|
||||||
|
## Deviations from the plan
|
||||||
|
|
||||||
|
- The plan's "preferred end state" converter is implemented, but the
|
||||||
|
metadata route still returns an array of per-instance datasets to keep
|
||||||
|
the existing `fetchMeta` contract intact.
|
||||||
|
- Private tags are excluded from metadata output. The plan did not require
|
||||||
|
them; excluding them keeps responses small and avoids site-private data.
|
||||||
|
- `convertToJpeg` remains untouched on the rendered/thumbnail paths.
|
||||||
|
- No integration test exercises the Fastify HTTP routes, because that path
|
||||||
|
imports `dicom-dimse-native` and needs a live PACS. Route-level frame
|
||||||
|
validation is covered through `parseFrameParam` and service errors.
|
||||||
|
- `skipLibCheck` was enabled in `tsconfig.json`. Without it `npm run build`
|
||||||
|
fails on a pre-existing `@types/glob`/`minimatch` mismatch in dependencies.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
- `npm run build` -> pass
|
||||||
|
- `npm test` -> 16 tests, 16 pass, 0 fail
|
||||||
|
- `npx eslint` on changed files -> no errors
|
||||||
|
|
||||||
|
Manual visual verification of a Jam-like CR fixture was not possible in this
|
||||||
|
environment: the original study data is not available. The synthetic CR
|
||||||
|
fixture proves no `40/80` window is fabricated, which is the stated root cause.
|
||||||
|
|
||||||
|
## Documentation Updates Needed
|
||||||
|
|
||||||
|
This repository has no `docs/` directory. Optional updates:
|
||||||
|
|
||||||
|
### README.md
|
||||||
|
|
||||||
|
Add a test section after the "Setup Instructions - source" block:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
* run the unit tests (uses node:test):
|
||||||
|
```npm test```
|
||||||
|
```
|
||||||
Reference in New Issue
Block a user