From f31dd0762135371e581f371f43980a805abfd45f Mon Sep 17 00:00:00 2001 From: padmanto Date: Fri, 18 Sep 2026 14:16:36 +0700 Subject: [PATCH] docs(report): add implementation notes for DICOMweb proxy recommendation --- report/01-dicom-web-proxy-recommendation.md | 125 ++++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 report/01-dicom-web-proxy-recommendation.md diff --git a/report/01-dicom-web-proxy-recommendation.md b/report/01-dicom-web-proxy-recommendation.md new file mode 100644 index 0000000..baa19c3 --- /dev/null +++ b/report/01-dicom-web-proxy-recommendation.md @@ -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``` +```