feat: package ontology + validators, docs, examples, and CI#1
feat: package ontology + validators, docs, examples, and CI#1louspringer wants to merge 13 commits intomainfrom
Conversation
…d CI\n\n- Python: pyproject + goblin_ontology with CLI (goblin-validate)\n- JS: @nkllon/goblin-ontology with validator and CLI\n- Bundled TTL/SHACL in both packages\n- Rendered goblin-map artifacts and linked in docs\n- Examples + numeric scenarios in energy-gradient\n- CI: validation, DOT render check, docs link check, JS build
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 17
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dataFormat = options.dataFormat ?? guessFormat(dataPath); | ||
| const shapesFormat = options.shapesFormat ?? "turtle"; | ||
|
|
||
| if (dataFormat !== "turtle") { | ||
| throw new Error( | ||
| `Unsupported data format '${dataFormat}' in JS validator yet; please provide Turtle (.ttl).` | ||
| ); | ||
| } | ||
| if (shapesFormat !== "turtle") { | ||
| throw new Error(`Unsupported shapes format '${shapesFormat}', expected 'turtle'.`); |
There was a problem hiding this comment.
JS validator rejects JSON-LD inputs despite advertised support
The Node validator claims to accept jsonld (the ValidateOptions type includes it and guessFormat returns jsonld for .json/.jsonld files), yet it immediately throws if dataFormat is anything other than "turtle". This means running the CLI or API on the provided JSON-LD example (examples/sample_score.json) fails before validation, unlike the Python validator which handles JSON-LD. Either support JSON-LD parsing or narrow the accepted types to avoid crashing users.
Useful? React with 👍 / 👎.
| const validator = new Validator(shapesDataset, { factory: DataFactory }); | ||
| const report = await validator.validate(dataDataset); |
There was a problem hiding this comment.
Node validator skips superclass shapes without RDFS inference
The JavaScript validator builds Validator without enabling any RDFS inference, while the shapes rely on class hierarchy (e.g., uiSh:GoblinScoreShape targets ui:ProblemClass to enforce the 0–1 range). The ontology defines ui:UIClosureIllusionProblem as a subclass of ui:ProblemClass, and the sample data only declares the subclass type. The Python validator explicitly requests inference='rdfs', so it checks the range; the Node validator will not and will accept out-of-range goblinScore values for subclass instances. Enable RDFS inference or materialize superclass types before validation to avoid false negatives.
Useful? React with 👍 / 👎.
- Move heavy checks to pre-push; add Docker-based hooks - Add Makefile and docker-compose to reproduce CI locally - Fix SHACL by removing dual domain on ui:forDimension; validation passes - Regenerate docs/goblin-map.* to satisfy dot-check - JS: set rdf-validate-shacl ^0.6.5, NodeNext module; build/typecheck pass
|
|
||
| ui:forDimension a owl:ObjectProperty ; | ||
| rdfs:label "for dimension" ; | ||
| rdfs:domain ui:ScoreComponent , ui:WeightAssignment ; |
There was a problem hiding this comment.
Bug: Bundled ontology files still contain removed domain declaration
The bundled copies of goblin-ontology.ttl in the Python and JS packages still declare rdfs:domain ui:ScoreComponent , ui:WeightAssignment for ui:forDimension, while the main ontology file removed this dual domain declaration to fix SHACL inference issues. This inconsistency means the packages ship the buggy version that causes SHACL validation failures, contradicting the fix documented in the CHANGELOG.
Additional Locations (1)
|
|
||
| Compute: | ||
| \[ | ||
| G_{\text{local}} \approx 0.18(0.95)+0.20(0.8)+0.15(0.7)+0.12(0.75)+0.10(0.7)+0.25(0.9)=0.833 |
There was a problem hiding this comment.
Bug: Incorrect arithmetic in energy gradient scenario calculations
The numerical scenarios contain arithmetic errors in the Goblin score calculations. Line 104 claims G_before = 0.808 but the correct sum is 0.809. Line 107 claims G_after = 0.724 but the correct sum is 0.755 (error of 0.031, significantly affecting the delta calculation). Line 120 claims G_local = 0.833 but the correct sum is 0.821. These errors make the example scenarios misleading for understanding the energy gradient model.
There was a problem hiding this comment.
Bug: Weight-dimension associations lost in DefaultWeights
The ui:DefaultWeights instance declares multiple ui:forDimension and ui:hasWeight values as flat properties on a single resource, which in RDF creates separate unlinked triples. There's no way to determine which weight value pairs with which dimension. Each dimension-weight pair needs its own ui:WeightAssignment instance (or a reified structure) to preserve the associations.
goblin-ontology.ttl#L412-L419
Lines 412 to 419 in 3a22e68
goblin_ontology/data/goblin-ontology.ttl#L412-L419
goblin/goblin_ontology/data/goblin-ontology.ttl
Lines 412 to 419 in 3a22e68
js/goblin-ontology/assets/goblin-ontology.ttl#L412-L419
goblin/js/goblin-ontology/assets/goblin-ontology.ttl
Lines 412 to 419 in 3a22e68
| idToNode[n.id] = n; | ||
| }); | ||
| const simulation = simulation_default(nodes).force("link", link_default(edges).id((d) => d.id).distance(120)).force("charge", manyBody_default().strength(-180)).force("center", center_default(width / 2, height / 2)).on("tick", () => { | ||
| link.attr("x1", (d) => idToNode[d.source].x).attr("y1", (d) => idToNode[d.source].y).attr("x2", (d) => idToNode[d.target].x).attr("y2", (d) => idToNode[d.target].y); |
There was a problem hiding this comment.
Bug: Incorrect link position calculation in force graph
The tick callback accesses link positions using idToNode[d.source] and idToNode[d.target], but after d3.forceLink initializes, d.source and d.target are already node object references, not ID strings. Using an object as a dictionary key will stringify it to "[object Object]", causing a lookup failure. The code redundantly creates an idToNode map when D3 already converts edge source/target to node references. The correct approach is accessing d.source.x directly instead of idToNode[d.source].x.
Additional Locations (1)
There was a problem hiding this comment.
Bug: Force link source/target accessed incorrectly
D3's forceLink replaces the source and target edge properties with references to node objects, but the code attempts to look them up in idToNode as if they were still IDs. When d.source (which is now a node object) is used as a key in idToNode[d.source], JavaScript converts it to the string "[object Object]", causing the lookup to fail and return undefined. This results in runtime errors when accessing .x on undefined. The fix is to directly access d.source.x and d.target.x instead of using the idToNode lookup.
web/src/main.ts#L105-L110
Lines 105 to 110 in edc364e
|
|
||
| # Run a single job with GHCR-backed act, e.g.: make act-job-ghcr JOB=js | ||
| act-job-ghcr: | ||
| bash -lc 'tools/ci/ghcr_login.sh && docker pull ghcr.io/nektos/act:latest >/dev/null && docker run --rm -v /var/run/docker.sock:/var/run/docker.sock -v "$$PWD":/github/workspace -v "$$HOME/.act":/root/.act -w /github/workspace ghcr.io/nektos/act:latest pull_request -P ubuntu-latest=catthehacker/ubuntu:act-latest -j "$${JOB}"' |
There was a problem hiding this comment.
Bug: Makefile target uses undefined shell variable
The act-job-ghcr target references $${JOB} which expands to ${JOB} in the shell, expecting a shell environment variable. However, the usage comment indicates JOB should be passed as a Make variable via make act-job-ghcr JOB=js. Make variables aren't automatically exported to the shell environment, so ${JOB} will be empty/undefined when the docker command executes, causing the -j flag to receive no job name.
|
Updated with:\n- CI: skip Pages upload under act\n- Regenerated docs/goblin-map.*\n- Local act suite passing |
There was a problem hiding this comment.
Bug: D3 force link source/target object-as-key lookup fails
The tick callback uses idToNode[d.source] and idToNode[d.target] to look up node coordinates. However, D3's forceLink mutates edge objects in place, replacing string ID values in source and target with actual node object references after initialization. Since objects used as map keys convert to "[object Object]", the lookup returns undefined, causing a runtime error when accessing .x and .y. The code should access d.source.x directly since d.source IS the node object after simulation initialization.
web/src/main.ts#L96-L101
Lines 96 to 101 in 14f21f9
docs/app.js#L3624-L3625
Lines 3624 to 3625 in 14f21f9
This PR adds packaging, validation, docs, examples, and CI for the Goblin score artifacts.\n\nWhat’s included:\n- Python (uv) package: pyproject + goblin_ontology with CLI (goblin-validate)\n- JS/TS package: @nkllon/goblin-ontology with validator and CLI\n- Bundled ontology and SHACL in both packages\n- Rendered goblin-map artifacts (SVG/PNG) and linked from docs\n- Examples: sample input and JSON-LD score\n- Energy-gradient note extended with numeric scenarios\n- CI: SHACL validation, DOT render diff check, docs link check, JS build\n\nAfter merge, consumers can:\n- Python: Validation Report
Conforms: True\n- Node: \n
Note
Packages the ontology with Python/Node SHACL validators and CLIs, adds a D3 docs app and examples, and wires CI workflows for validation, build, and artifact checks.
rdfs:domainonui:forDimensioningoblin-ontology.ttl.goblin_ontology/data/andjs/goblin-ontology/assets/.goblin-ontologywithgoblin_ontologymodule andgoblin-validateCLI (RDFlib + PySHACL).@nkllon/goblin-ontologywith SHACL validation API andgoblin-validateCLI.docs/index.html,docs/app.js,docs/style.css) anddocs/goblin.jsondata.docs/goblin-map.svg/pngand adddocs/ttl-graph.*.examples/).ci.ymltweak for Pages artifact.tools/*scripts for local parity.Written by Cursor Bugbot for commit 14f21f9. This will update automatically on new commits. Configure here.