Skip to content

refactor: consolidate tooling, pipeline blueprint, and asset alignment#1

Open
SevenOfNine-ai wants to merge 4 commits intoASCS-eV:mainfrom
SevenOfNine-ai:feature/consolidate-tooling
Open

refactor: consolidate tooling, pipeline blueprint, and asset alignment#1
SevenOfNine-ai wants to merge 4 commits intoASCS-eV:mainfrom
SevenOfNine-ai:feature/consolidate-tooling

Conversation

@SevenOfNine-ai
Copy link

Summary

Full consolidation of the scenario-asset-example repository: tooling, pipeline, generated asset structure, and submodule alignment. Analogous to ASCS-eV/hd-map-asset-example#1.

Root Repository Changes

Build System & Tooling

  • Add Makefile as central command center (setup, generate, validate, clean, help)
  • Add pre-commit hooks with SHACL/JSON-LD validation (replaces black/isort/flake8)
  • Remove orphan .flake8 and unused pyproject.toml
  • Add .gitattributes enforcing LF line endings
  • Add .markdownlint.yaml for consistent Markdown style
  • Align make help with debug logging and deterministic mode hints

Asset Structure (EVES-003)

  • Remove hand-crafted asset/ directory (replaced by pipeline-generated output)
  • Add generated/input/ with input_manifest.json (JSON-LD), simulation data (.xosc), media, docs, LICENSE
  • Pipeline output goes to generated/output/ (gitignored)

Submodules

  • Consolidate into submodules/ (sl-5-8-asset-tools, EVES)
  • Remove stale root ontology-management-base pointer (nested inside sl-5-8)
  • EVES submodule added via HTTPS

Documentation

  • Rewrite README.md with setup/usage/structure/FAQ docs
  • Add .github/copilot-instructions.md

Deferred

  • release.yml update to softprops/action-gh-release@v2 requires workflow scope — will follow up in a separate commit once the CI token is refreshed.

Diff Stats

 24 files changed, 1010 insertions(+), 1185 deletions(-)

Key files added

  • Makefile — central command center
  • generated/input/input_manifest.json — JSON-LD input manifest (v5 context)
  • .github/copilot-instructions.md — AI agent instructions
  • .markdownlint.yaml — Markdown lint config

Key files removed

  • asset/ — entire hand-crafted directory (replaced by pipeline)
  • .flake8, pyproject.toml — orphan Python config
  • ontology-management-base — root submodule (now nested in sl-5-8)

Blueprint Reference

This PR follows the same pattern established in ASCS-eV/hd-map-asset-example#1, adapted for OpenSCENARIO scenario assets instead of OpenDRIVE HD maps.

- Remove root ontology-management-base submodule (now via recursive init
  through sl-5-8-asset-tools)
- Add Makefile as central command center (OMB pattern)
  - OS detection for cross-platform support (Windows + Unix)
  - Subcommand groups: generate clean, wizard stop, clean all
- Add sl-5-8-asset-tools + EVES as submodules under submodules/
- Remove hand-crafted asset/ directory (replaced by pipeline-generated output)
- Add generated/input/ with input_manifest.json (JSON-LD) and staged
  simulation data, media, docs, LICENSE
- Pipeline output goes to generated/output/ (gitignored)
- Add .gitattributes enforcing LF line endings
- Add .markdownlint.yaml config
- Replace black/isort/flake8 pre-commit hooks with SHACL/JSON-LD validation
- Add .github/copilot-instructions.md
- Complete README rewrite with setup/usage/structure/FAQ docs
- Remove orphan .flake8 and unused pyproject.toml

Note: release.yml update (softprops/action-gh-release@v2) deferred to
a follow-up commit requiring workflow scope.

Signed-off-by: Seven of Nine <github.com.entitle117@passmail.com>
- Fix OMB path: submodules/ → external/ (matches actual submodule layout)
- Fix setup: install deps via requirements.txt + editable OMB install
  (submodule has no setup target or pyproject.toml to delegate to)
- Fix generate: call asset_extraction.main directly with manifest
  conversion script (submodule has no generate target)
- Fix validate: check for correct output filenames
  (manifest_reference.json, scenario_instance.json)
- Add scripts/convert_manifest.py to bridge input_manifest.json (JSON-LD)
  to uploadedFiles.json (pipeline format)
- Simplify clean target (no submodule delegation needed)
- Add generated intermediates to .gitignore

Signed-off-by: Seven of Nine <github.com.entitle117@passmail.com>
@SevenOfNine-ai
Copy link
Author

Summary

The refactor is directionally correct: the Makefile now points at the right nested OMB location, the staged input_manifest.json is internally consistent, and convert_manifest.py emits the uploadedFiles.json shape that sl-5-8-asset-tools expects. However, I do not think this is merge-ready yet because the repository's release automation still targets the deleted hand-crafted asset/ layout, and several docs/instructions still describe paths/files that the pinned pipeline does not actually produce.

Findings

🔴 blocking

  • Release automation will break after merge. .github/workflows/release.yml:13-18 still checks out without submodules, and :28-31 still does cd asset + zips the removed asset/ directory. This PR deletes asset/, so the first tagged release will fail immediately. README/Copilot already claim the workflow is make setup && make generate && make validate, but the workflow file has not been updated yet. I would not merge until the workflow is brought in line with the new pipeline (or the docs stop claiming it already is).

🟡 suggestion

  • README / Copilot still contain several stale pipeline details. Examples:
    • README.md:45-53 and .github/copilot-instructions.md:49-54 describe output files as manifest.json / metadata/scenario.json, but the pinned sl-5-8-asset-tools still generates manifest_reference.json / metadata/scenario_instance.json.
    • README.md:110-112 and .github/copilot-instructions.md:29 place ontology-management-base under submodules/ontology-management-base, while Makefile:11 and the pinned sl-5-8-asset-tools tree use external/ontology-management-base.
    • README.md:76-77 and .github/copilot-instructions.md:17-18 describe a working wizard flow, but Makefile:143-145 is explicitly only a stub for this asset type.
    • README.md:137-142 documents a manual pipeline invocation that does not correspond to an installable module in the pinned sl-5-8-asset-tools repo.
  • Dev bootstrap is not fully reproducible yet. Makefile:67 tries to run python -m pre_commit install, but pre-commit is not installed by the checked requirements set, so that silently no-ops unless the user already has it. In addition, README.md:9-20 does not list Node/npm even though make lint-md and the markdown pre-commit hook rely on npx (Makefile:86-95, .pre-commit-config.yaml:11-16). I would either install these explicitly in make setup or document them.
  • Pre-commit is stricter than the repo layout suggests. .pre-commit-config.yaml:5-10 always runs make validate, and Makefile:107 exits non-zero when generated/output/ does not exist. On a fresh clone, that means even a docs-only commit fails until make generate has been run once. If that is intentional, it should be documented; otherwise I would gate that hook on generated output or replace it with a lighter manifest sanity check.

🟢 looks good

  • Makefile:10-11 uses submodules/sl-5-8-asset-tools/external/ontology-management-base, which matches the pinned sl-5-8-asset-tools commit and fixes the stale submodules/ assumption from the hd-map example.
  • generated/input/input_manifest.json uses the v5 manifest context, the listed files all resolve to real files in generated/input/, and application/x-xosc is consistent with the upstream pipeline MIME mapping for simulation-data .xosc.
  • scripts/convert_manifest.py maps the JSON-LD input manifest to the expected uploadedFiles.json categories/types (Asset, Document, Image, License) and the absolute-path resolution makes the top-level make generate flow robust.
  • .gitmodules looks sane: only submodules/sl-5-8-asset-tools and submodules/EVES are direct submodules, with ontology-management-base correctly left nested inside sl-5-8.

Verdict: Request Changes

— Reviewed by GPT 5.4 (automated code review)

- Replace manifest.json → manifest_reference.json in output diagrams
- Replace metadata/scenario.json → metadata/scenario_instance.json
- Fix OMB path: submodules/ → external/ontology-management-base
- Add Node.js/npm to prerequisites (needed for markdownlint)
- Add wizard Podman requirement note
- Update Option B manual pipeline to match actual invocation
- Update release workflow description in copilot-instructions
- Fix asset structure conventions in copilot-instructions

Signed-off-by: Seven of Nine <github.com.entitle117@passmail.com>
- Add 'pre-commit' to pip install in setup target
- Guard validate pre-commit hook: skip if no generated output exists
  (prevents hook failure on fresh clones before 'make generate')

Signed-off-by: Seven of Nine <github.com.entitle117@passmail.com>
@SevenOfNine-ai
Copy link
Author

Review Findings — Fix Summary

All review findings have been addressed. Two commits pushed, one blocked (requires Carlo).

✅ Resolved (pushed)

  • Output filenames: manifest.jsonmanifest_reference.json and metadata/scenario.jsonmetadata/scenario_instance.json in README + copilot-instructions (d301255)
  • OMB nested path: submodules/ontology-management-baseexternal/ontology-management-base in README diagram + copilot-instructions (d301255)
  • Wizard flow: Added note that make wizard requires Podman and is optional/experimental (d301255)
  • Manual pipeline (Option B): Updated to match actual asset_extraction.main invocation pattern with convert_manifest.py prep step (d301255)
  • Node.js prerequisite: Added to Prerequisites table in README (d301255)
  • pre-commit install: Added pip install pre-commit to Makefile setup target (036c2de)
  • Validate hook guard: Pre-commit validate hook now skips gracefully if generated/output/ does not exist yet (036c2de)
  • Repo structure diagram: Fixed submodules/external/ for ontology-management-base path (d301255)
  • Copilot-instructions asset conventions: Fixed manifest.jsonmanifest_reference.json and metadata/scenario.jsonmetadata/scenario_instance.json in Key Conventions section (d301255)

🔴 Blocked — requires Carlo

  • release.yml: Commit 8209af9 updates the release workflow from Node.js/zip to Python/Make pipeline, but cannot be pushed — GitHub rejects it because the OAuth token lacks workflow scope. Carlo must either:
    1. Push this commit manually: git fetch fork && git push fork feature/consolidate-tooling (from a local clone with workflow-scoped token), or
    2. Update the GitHub OAuth token to include workflow scope

The release.yml commit is the tip of the local branch, sitting on top of all pushed work. Everything else is live on the PR.

@SevenOfNine-ai
Copy link
Author

🔴 Action needed: release.yml update

The updated release.yml cannot be pushed via the API — GitHub requires workflow OAuth scope for any workflow file modification, and the GitHub App token only has access to SevenOfNine-labs repos, not the SevenOfNine-ai fork.

Option A — Quickest (run on the Mac Mini):

cd ~/projects/scenario-asset-example  # or wherever the local clone is
gh auth refresh -h github.com -s workflow
git push fork feature/consolidate-tooling

Option B — Apply manually:
Replace .github/workflows/release.yml with this content and push:

name: Create Release

on:
  push:
    tags:
      - 'v*.*.*'

permissions:
  contents: write

jobs:
  release:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout repository
      uses: actions/checkout@v4
      with:
          lfs: true
          submodules: recursive

    - name: Checkout LFS objects
      run: git lfs checkout

    - name: Set up Python
      uses: actions/setup-python@v5
      with:
        python-version: '3.12'

    - name: Setup, generate, and validate
      run: |
        make setup
        make generate
        make validate

    - name: Create Release and Upload Asset
      uses: softprops/action-gh-release@v2
      with:
        files: "generated/output/**/*.zip"
        generate_release_notes: true

Seven of Nine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant