Conversation
There was a problem hiding this comment.
The issue asks for a new GitHub directory for Skull (e.g., DataSkull) with skull.json and images accessible via raw GitHub URL. In this PR, Skull data lives under boneset-api/data/final_skull.json. I don’t see a DataSkull/ folder or a raw-URL fetch path added. This misses the “store in GitHub for raw access” requirement and makes prod dependent on bundling data in the server image.
Lets maintain a consistant image paths.Because Skull is served from a local JSON file, the image paths inside final_skull.json must be valid from the app’s point of view (either local static paths you serve, or raw GitHub URLs). The PR doesn’t show that wiring (static express folder or new GitHub paths). Verify these resolve in the UI.
Tips to note
To fix this, clean /combined-data by replacing the handler with a single flow: load the pelvis from the existing GitHub JSONs, push its boneset/bones/subbones, load the skull with await loadLocalBoneset("skull"), push its boneset/bones/subbones, and return { bonesets, bones, subbones } once; this removes the duplicate bonesets declaration and the nested “fetch pelvis inside a pelvis loop” bug.
Next, unify the cache layer by keeping bonesetCache = new Map() with loadLocalBoneset(id) and deleting the old cachedBoneset and loadBoneset() so all local reads go through the new function. Fix the /api/description duplication by removing the second for (const line of lines) so descriptions render once, and keep validation on boneId and ALLOWED_BONESETS.has(bonesetId).
To meet the “DataSkull” acceptance criteria, create DataSkull/ in the repo (parallel to DataPelvis/), commit boneset/skull.json and all images there, and add Skull GitHub base URLs like:
---> const SKULL_REPO = "https://raw.githubusercontent.com/oss-slu/DigitalBonesBox/data/DataSkull/";
---> const SKULL_JSON_URL = ${SKULL_REPO}boneset/skull.json; ---> const SKULL_BONES_DIR = ${SKULL_REPO}bones/`;.
For consistency, have /combined-data pull both pelvis and skull from raw GitHub URLs. Finally, verify the dropdown by confirming /combined-data returns bonesetscontaining both{ id: "bony_pelvis" } and { id: "skull" }`, so the UI shows both options.
|
|
||
| // Unchanged: used by the dropdowns in the current UI | ||
| // Unchanged pelvis aggregation + add Skull from local final_skull.json | ||
| app.get("/combined-data", async (_req, res) => { |
There was a problem hiding this comment.
The route re-declares bonesets and mixes old pelvis logic with the new flow. You initialize bonesets twice and start a loop on bonesetData.bones while also fetching pelvis again inside that loop. This will either throw or produce wrong results. Clean this to one clear flow: build pelvis, then skull, then return
boneset-api/server.js
Outdated
|
|
||
| try { | ||
| const set = await loadBoneset(); | ||
| const set = await loadLocalBoneset(bonesetId); // <- IMPORTANT |
There was a problem hiding this comment.
You introduce a bonesetCache and a new loadLocalBoneset(id) while also keeping the old single-file cache fields (cachedBoneset, old loadBoneset() pattern). Pick one approach and remove the leftovers so readers aren’t confused.
| // Safe path + allowlist + rate limit to fetch the full local JSON | ||
| app.get("/api/boneset/:bonesetId", bonesetLimiter, async (req, res) => { | ||
| const { bonesetId } = req.params; | ||
|
|
There was a problem hiding this comment.
/api/description renders lines twice, The code appends the description lines in a loop, then immediately appends them again in a second loop. You’ll get duplicated (li) items.
|
update @UcheWendy Done (per acceptance criteria & review):
Current blocker I’m fixing: images not rendering
|
|
Scope too large |








Description
Partially addresses #127 – Content Pipeline: Process and Integrate a New Boneset (Skull).
This PR sets up the Skull extraction pipeline and checks in the raw, per-slide outputs we’ll use to build the final
skull.json. It adds the folder scaffolding, unzips the PPTX into XML, and runs our extraction scripts to produce images plus JSON annotations/descriptions.Dependencies: Python 3 stdlib only (
xml.etree,json,os). No new npm deps.1. What was Changed
data_extraction/skull/{ppt,annotations,images}/.data_extraction/skull/ppt/unzipped/….data_extraction/extract_ppt_annotations.py→ per-slide*_annotations.json+ extracted slide images.data_extraction/Extract_Bone_Descriptions.py→ per-slide*_Descriptions.json.*_annotations.jsonfiles (slide 1 has no image/labels).*_Descriptions.jsonfiles (one per slide).data_extraction/skull/images/slide*/….2. Why it was Changed
Kick off the content pipeline for the Skull boneset and produce clean, machine-readable labels and descriptions. This unblocks creation of a single
skull.jsonand later API/UI integration alongside Bony Pelvis.3. How it was Changed
Bone Box (Skull).pptx→data_extraction/skull/ppt/skull.pptx.data_extraction/skull/ppt/unzipped/to parseppt/slides/*.xml, relationships, and media.extract_ppt_annotations.py)…/ppt/unzipped/ppt/slides…/ppt/unzipped/ppt/slides/_rels…/ppt/unzipped/ppt/mediadata_extraction/skull/images&data_extraction/skull/annotationsr:embed→ media, wrote images toimages/slideN/…, savedslideN_annotations.json.No images found… Skipping.Extract_Bone_Descriptions.py){ id, name, description: [...] }toslideN_Descriptions.json.*_annotations.json: 32*_Descriptions.json: 33slide3_annotations.json) for sensible label coordinates/text.4. Screenshots (if applicable)
N/A (no UI/API changes in this PR). Folder tree and terminal logs available upon request.
Next steps (tracked in #127)
skull.jsonwith full hierarchy (bones, sub-bones, descriptions, annotations, image paths).server.jsto load from the new raw URL.