jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore(ci): sprint 38 — orphan-tickets CLI + semver schema stamps #139

Closed
jpmschweitzer wants to merge 0 commits from sprint-38/ci into main
pull from: sprint-38/ci
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-05-02 10:12:59 +02:00
Owner
Copy Link
Copy Source

Summary

Sprint 38 CI — two tooling improvements:

  • #887: decisions-orphan-tickets CLI — new tooling/db/decision orphan-tickets subcommand that scans tickets with a decision_ref not matching any decision in the DB. Surfaces silently orphaned tickets from typo'd or renumbered D-IDs. Makefile target: make decisions-orphan-tickets.
  • #888: meta.schema_version → monotonic semver — replaces SHA-1 hash in meta.schema_version with an orderable semver string ("1.0.0"). Old SHA preserved in new schema_sha column for tamper detection. check-systems-db-stamp now validates semver format and rejects legacy SHA-hex values. Enables future savegame migration lineage (Phase 5+).

Changed files

File Change
tooling/db/decisions_sync.py New orphan-tickets subcommand
Makefile New decisions-orphan-tickets target
tooling/economy-db/import_economics.py SCHEMA_VERSION constant, semver stamp, schema_sha migration
tooling/planet-gen/generate_atlas.py SCHEMA_VERSION constant, semver stamp, migration
tooling/check-systems-db-stamp Semver validation, schema_sha awareness
server/data/systems-schema.sql meta table DDL updated with schema_sha column
server/data/systems.db Regenerated with semver stamps
.claude/rules/asset-pipeline.md Documentation updated
CHANGELOG.md Updated

Verification

  • make decisions-orphan-tickets — runs cleanly, found 14 orphan tickets (all pre-existing stale refs)
  • make regen-db — succeeds
  • make check-systems-db — reports OK — 2 generator(s) up to date
  • ruff check tooling/ — all checks passed
  • Pre-push hook — all checks passed
## Summary Sprint 38 CI — two tooling improvements: - **#887: decisions-orphan-tickets CLI** — new `tooling/db/decision orphan-tickets` subcommand that scans tickets with a `decision_ref` not matching any decision in the DB. Surfaces silently orphaned tickets from typo'd or renumbered D-IDs. Makefile target: `make decisions-orphan-tickets`. - **#888: meta.schema_version → monotonic semver** — replaces SHA-1 hash in `meta.schema_version` with an orderable semver string (`"1.0.0"`). Old SHA preserved in new `schema_sha` column for tamper detection. `check-systems-db-stamp` now validates semver format and rejects legacy SHA-hex values. Enables future savegame migration lineage (Phase 5+). ## Changed files | File | Change | |------|--------| | `tooling/db/decisions_sync.py` | New `orphan-tickets` subcommand | | `Makefile` | New `decisions-orphan-tickets` target | | `tooling/economy-db/import_economics.py` | `SCHEMA_VERSION` constant, semver stamp, `schema_sha` migration | | `tooling/planet-gen/generate_atlas.py` | `SCHEMA_VERSION` constant, semver stamp, migration | | `tooling/check-systems-db-stamp` | Semver validation, `schema_sha` awareness | | `server/data/systems-schema.sql` | `meta` table DDL updated with `schema_sha` column | | `server/data/systems.db` | Regenerated with semver stamps | | `.claude/rules/asset-pipeline.md` | Documentation updated | | `CHANGELOG.md` | Updated | ## Verification - `make decisions-orphan-tickets` — runs cleanly, found 14 orphan tickets (all pre-existing stale refs) - `make regen-db` — succeeds - `make check-systems-db` — reports `OK — 2 generator(s) up to date` - `ruff check tooling/` — all checks passed - Pre-push hook — all checks passed
jpmschweitzer added 3 commits 2026-05-02 10:13:00 +02:00
chore(db): add decisions-orphan-tickets CLI (#887) ffa638a46c 
New `tooling/db/decision orphan-tickets` subcommand scans tickets with
a decision_ref that doesn't match any row in the decisions table.
Surfaces silently orphaned tickets from typo'd or renumbered D-IDs.
Makefile target: `make decisions-orphan-tickets`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(db): switch meta.schema_version to monotonic semver (#888) 5aa998cb86 
Replace SHA-1 hash in meta.schema_version with an orderable semver
string ("1.0.0"). SHA preserved in new schema_sha column for tamper
detection. Enables savegame migration lineage in Phase 5+ — saves can
record their schema version and determine which migrations to apply.

Updated both generators, check-systems-db-stamp validation (rejects
old SHA-hex values), schema DDL, and asset-pipeline docs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog 7cc1306996 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-05-02 10:20:57 +02:00
Author
Owner
Copy Link
Copy Source

Review: sprint-38/ci → main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Summary: Core logic is solid — SQL queries are correct, semver validation is well-constructed, and the two-pass sync approach is sound. Three issues need addressing.

# File Issue
1 tooling/db/decisions_sync.py refs_created counter increments unconditionally after INSERT OR IGNORE (always reports N refs even if 0 were new). Dead except IntegrityError block — INSERT OR IGNORE never raises. Fix: check cursor.rowcount > 0 before incrementing, remove dead except.
2 tooling/db/decisions_sync.py + tooling/planet-gen/generate_atlas.py SCHEMA_VERSION is two independent constants with no enforcement they stay in sync. First bump following the docs (which only mention import_economics.py) will silently diverge.
3 tooling/db/decision (wrapper) Usage comment omits show and orphan-tickets subcommands — undiscoverable from the wrapper.

Tyre (Architecture): REQUEST_CHANGES

Summary: Both tickets are sound in intent. The semver migration is well-conceived — the "old SHA-1 forces regen-db" path via bad_version is correct behavior. Three architectural issues to address.

# File Issue
1 import_economics.py + generate_atlas.py SCHEMA_VERSION is duplicated as two independent literals. Docs say to bump only import_economics.py. Extract to a shared module, or add a cross-row equality check to the stamp tool.
2 tooling/planet-gen/generate_atlas.py Bare except Exception: pass in ensure_atlas_schema swallows all errors (locked DB, permissions, malformed SQL). Should match import_economics.py's _add_column pattern: catch sqlite3.OperationalError, re-raise if not "duplicate column".
3 tooling/check-systems-db-stamp No cross-generator schema_version agreement check. Two generators could write different versions and it passes silently. Add a post-validation equality assertion.

Verdict: CHANGES REQUESTED

Deduplicated issues (5 unique):

  1. refs_created counter inflation + dead except block in decisions_sync.py
  2. SCHEMA_VERSION divergence risk — extract to shared module OR add cross-row check to stamp tool
  3. Bare except Exception: pass in generate_atlas.py — narrow to OperationalError + "duplicate column" check
  4. check-systems-db-stamp needs cross-generator version agreement assertion
  5. tooling/db/decision wrapper usage block missing show and orphan-tickets
## Review: sprint-38/ci → main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES **Summary:** Core logic is solid — SQL queries are correct, semver validation is well-constructed, and the two-pass sync approach is sound. Three issues need addressing. | # | File | Issue | |---|------|-------| | 1 | `tooling/db/decisions_sync.py` | `refs_created` counter increments unconditionally after `INSERT OR IGNORE` (always reports N refs even if 0 were new). Dead `except IntegrityError` block — INSERT OR IGNORE never raises. Fix: check `cursor.rowcount > 0` before incrementing, remove dead except. | | 2 | `tooling/db/decisions_sync.py` + `tooling/planet-gen/generate_atlas.py` | `SCHEMA_VERSION` is two independent constants with no enforcement they stay in sync. First bump following the docs (which only mention `import_economics.py`) will silently diverge. | | 3 | `tooling/db/decision` (wrapper) | Usage comment omits `show` and `orphan-tickets` subcommands — undiscoverable from the wrapper. | ### Tyre (Architecture): REQUEST_CHANGES **Summary:** Both tickets are sound in intent. The semver migration is well-conceived — the "old SHA-1 forces regen-db" path via `bad_version` is correct behavior. Three architectural issues to address. | # | File | Issue | |---|------|-------| | 1 | `import_economics.py` + `generate_atlas.py` | `SCHEMA_VERSION` is duplicated as two independent literals. Docs say to bump only `import_economics.py`. Extract to a shared module, or add a cross-row equality check to the stamp tool. | | 2 | `tooling/planet-gen/generate_atlas.py` | Bare `except Exception: pass` in `ensure_atlas_schema` swallows all errors (locked DB, permissions, malformed SQL). Should match `import_economics.py`'s `_add_column` pattern: catch `sqlite3.OperationalError`, re-raise if not "duplicate column". | | 3 | `tooling/check-systems-db-stamp` | No cross-generator `schema_version` agreement check. Two generators could write different versions and it passes silently. Add a post-validation equality assertion. | ### Verdict: CHANGES REQUESTED **Deduplicated issues (5 unique):** 1. `refs_created` counter inflation + dead except block in `decisions_sync.py` 2. `SCHEMA_VERSION` divergence risk — extract to shared module OR add cross-row check to stamp tool 3. Bare `except Exception: pass` in `generate_atlas.py` — narrow to `OperationalError` + "duplicate column" check 4. `check-systems-db-stamp` needs cross-generator version agreement assertion 5. `tooling/db/decision` wrapper usage block missing `show` and `orphan-tickets`
jpmschweitzer added 1 commit 2026-05-02 10:29:33 +02:00
fix(ci): address PR #139 review — 5 issues + source list sync 873746ff8b 
1. decisions_sync.py: fix refs_created inflation (check rowcount),
   remove dead IntegrityError except block
2. Extract SCHEMA_VERSION to shared tooling/schema_version.py —
   both generators import from single source of truth
3. generate_atlas.py: narrow bare except to OperationalError +
   "duplicate column" check
4. check-systems-db-stamp: add cross-generator schema_version
   agreement assertion (defense-in-depth)
5. decision wrapper: add show + orphan-tickets to usage text
6. Add schema_version.py to all three source watch lists
   (GENERATOR_SOURCES, IMPORT_ECONOMICS_SOURCES, generate_atlas
   _write_stamp) — prevents silent staleness on version bump

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-05-02 17:28:47 +02:00
Author
Owner
Copy Link
Copy Source

Review Round 2: sprint-38/ci → main

Previous issues: ALL FIXED

# Issue Status
1 refs_created counter + dead except FIXED
2 SCHEMA_VERSION duplicated FIXED (extracted to tooling/schema_version.py)
3 Bare except Exception: pass in generate_atlas FIXED (narrowed to OperationalError + "duplicate column")
4 No cross-generator agreement check FIXED (cross-row equality assertion added)
5 Wrapper usage missing subcommands FIXED

New issue (both reviewers)

.claude/rules/asset-pipeline.md and server/data/systems-schema.sql still reference tooling/economy-db/import_economics.py as the home of SCHEMA_VERSION. The constant now lives in tooling/schema_version.py. Three-line fix:

  1. asset-pipeline.md line ~55: update path
  2. asset-pipeline.md line ~186: update path
  3. systems-schema.sql meta table comment: update path

Verdict: CHANGES REQUESTED (minor — doc path references only)

## Review Round 2: sprint-38/ci → main ### Previous issues: ALL FIXED | # | Issue | Status | |---|-------|--------| | 1 | `refs_created` counter + dead except | FIXED | | 2 | `SCHEMA_VERSION` duplicated | FIXED (extracted to `tooling/schema_version.py`) | | 3 | Bare `except Exception: pass` in generate_atlas | FIXED (narrowed to OperationalError + "duplicate column") | | 4 | No cross-generator agreement check | FIXED (cross-row equality assertion added) | | 5 | Wrapper usage missing subcommands | FIXED | ### New issue (both reviewers) `.claude/rules/asset-pipeline.md` and `server/data/systems-schema.sql` still reference `tooling/economy-db/import_economics.py` as the home of `SCHEMA_VERSION`. The constant now lives in `tooling/schema_version.py`. Three-line fix: 1. `asset-pipeline.md` line ~55: update path 2. `asset-pipeline.md` line ~186: update path 3. `systems-schema.sql` meta table comment: update path ### Verdict: CHANGES REQUESTED (minor — doc path references only)
jpmschweitzer added 1 commit 2026-05-02 17:30:22 +02:00
fix(ci): update 3 doc references to tooling/schema_version.py 3489edf6ec 
asset-pipeline.md (2 locations) and systems-schema.sql still pointed
at import_economics.py as the SCHEMA_VERSION home after the extraction
to a shared module.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer closed this pull request 2026-05-02 18:10:06 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#139