Skip to content

Unify Dijkstra-era placeholder messages under TODO Dijkstra#1187

Merged
Jimbo4350 merged 2 commits intomasterfrom
jordan/dijkstra-placeholder-unification
Apr 27, 2026
Merged

Unify Dijkstra-era placeholder messages under TODO Dijkstra#1187
Jimbo4350 merged 2 commits intomasterfrom
jordan/dijkstra-placeholder-unification

Conversation

@Jimbo4350
Copy link
Copy Markdown
Contributor

Changelog

- description: |
    Unify Dijkstra-era placeholder `error` messages and `TODO` comments
    under the single greppable token `TODO Dijkstra:`, so
    `grep -r 'TODO Dijkstra'` locates every Dijkstra-era placeholder in
    cardano-api in one shot. No behaviour change.
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
   - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...
# uncomment at least one main project this PR is associated with
  projects:
   - cardano-api
  # - cardano-api-gen
  # - cardano-rpc
  # - cardano-wasm

Context

Follow-up to #1175. A reviewer there noted that Dijkstra-era placeholders in cardano-node use three different conventions, so no single grep catches them all. The same problem exists in cardano-api: 27 error stubs mentioning Dijkstra, spread across 5+ phrasings (not supported, is not yet supported, not supported yet, currently not supported, is not active yet), plus 4 -- TODO comments in various forms. grep 'TODO.*Dijkstra' missed the error stubs; grep 'Dijkstra era' missed the TODOs; no single grep caught all 31.

This PR unifies 30 of them (see exclusions below) under the literal contiguous token TODO Dijkstra so grep -rn 'TODO Dijkstra' cardano-api/src now returns every Dijkstra-era placeholder a future maintainer needs to audit when Dijkstra-era support lands.

Format applied

  • Error stubs: error \"TODO Dijkstra: <function>: <brief reason>\" — preserves existing descriptive context so runtime stack traces remain self-explanatory.
  • TODO comments: -- TODO Dijkstra: <brief reason> — colon is after Dijkstra, not between TODO and Dijkstra, otherwise grep 'TODO Dijkstra' misses.

Sites touched (30)

  • Case combinators (7): Era/Internal/Case.hs:{55,73,90,107,124,141,158}
  • Eon constraints (5): Era/Internal/Eon/{AllegraEraOnwards,MaryEraOnwards,BabbageEraOnwards,ConwayEraOnwards,ShelleyBasedEra}.hs
  • Old API (7): Certificate/Internal.hs:{829,846}, Tx/Internal/Body.hs:{2665,3107}, Tx/Internal/Body/Lens.hs:172, Plutus/Internal/Script.hs:{1429,1463}
  • Experimental API (6): Experimental/Tx/Internal/Fee.hs:{431,1545}, Experimental/Tx/Internal/AnyWitness.hs:128, Experimental/Tx/Internal/Certificate/Compatible.hs:74, Experimental/Tx/Internal/BodyContent/New.hs:{197,849}
  • Serialisation orphan (1): Internal/Orphans/Serialisation.hs:273 (error) + L271 (adjacent TODO)
  • TODO comments (4): Internal/Orphans/Serialisation.hs:271, LedgerState.hs:{1168,1514,1515}

Intentional exclusions

  • Tx/Internal/Body.hs:2858toScriptIndexDijkstra: unexpected DijkstraGuarding at index N is a runtime invariant assertion, not a "Dijkstra era not yet supported" placeholder. DijkstraGuarding is a ledger-side constructor with no corresponding ScriptWitnessIndex case; the error fires only if the ledger ever hands us one. Fixing that would mean adding a new ScriptWitnessIndex variant (a breaking change) — different kind of work from everything else on the list.
  • Plutus/Internal/Script.hs:1302toShelleyScript: PlutusV4 not implemented yet. is a PlutusV4 stub that blocks on ledger exposing a PlutusV4 constructor. Will be tracked as a dedicated issue rather than prefixed with TODO Dijkstra (it's PlutusV4-gated, not Dijkstra-gated).

How to trust this PR

  • Verify the greppability claim:
    grep -rn 'TODO Dijkstra' cardano-api/src | wc -l     # → 30
    grep -rn 'is not yet supported\|currently not supported\|is not active yet\|not supported yet' cardano-api/src  # → no Dijkstra hits
    grep -rn 'DijkstraEra is not supported\|DijkstraEra not supported\|Dijkstra era is not\|Dijkstra era not supported' cardano-api/src  # → no hits
    grep -rn -- '-- TODO: Dijkstra\|TODO: Build dummy dijkstra\|TODO: fix this instance when the Dijkstra' cardano-api/src  # → no hits
    grep -n 'unexpected DijkstraGuarding' cardano-api/src/Cardano/Api/Tx/Internal/Body.hs  # → 1 hit (intentionally preserved)
    
  • Only strings/comments changed — no type signatures or logic touched. Diff is 30 insertions / 30 deletions on the .hs files.
  • Builds clean: cabal build cardano-api -j4.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • Self-reviewed the diff

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Standardizes Dijkstra-era placeholder error messages and TODO comments across cardano-api under a single greppable token (TODO Dijkstra:), to make future cleanup/audits easy and reliable without changing behavior.

Changes:

  • Replaces varied Dijkstra placeholder error strings with error "TODO Dijkstra: <function>: <reason>".
  • Normalizes Dijkstra-related TODO comments to the -- TODO Dijkstra: ... format.
  • Adds a Herald changelog fragment marking the change as maintenance.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cardano-api/src/Cardano/Api/Tx/Internal/Body/Lens.hs Unifies Dijkstra-era placeholder error string for lens accessor.
cardano-api/src/Cardano/Api/Tx/Internal/Body.hs Unifies Dijkstra-era placeholder error strings in tx body construction/vote extraction.
cardano-api/src/Cardano/Api/Plutus/Internal/Script.hs Unifies Dijkstra-era placeholder error strings in script conversions.
cardano-api/src/Cardano/Api/LedgerState.hs Normalizes Dijkstra-related TODO comments around genesis/hard-fork trigger handling.
cardano-api/src/Cardano/Api/Internal/Orphans/Serialisation.hs Normalizes Dijkstra TODO comment and placeholder ToJSON error for Dijkstra ApplyTxError.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/Fee.hs Unifies Dijkstra-era placeholder error strings in fee/auto-balance paths.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/Certificate/Compatible.hs Unifies Dijkstra-era placeholder error string in certificate construction.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/BodyContent/New.hs Unifies Dijkstra-era placeholder error strings in unsigned tx and vote witness extraction.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/AnyWitness.hs Unifies Dijkstra-era placeholder error string for Plutus datum extraction.
cardano-api/src/Cardano/Api/Era/Internal/Eon/ShelleyBasedEra.hs Unifies Dijkstra-era placeholder error string in constraints helper.
cardano-api/src/Cardano/Api/Era/Internal/Eon/MaryEraOnwards.hs Unifies Dijkstra-era placeholder error string in constraints helper.
cardano-api/src/Cardano/Api/Era/Internal/Eon/ConwayEraOnwards.hs Unifies Dijkstra-era placeholder error string in constraints helper.
cardano-api/src/Cardano/Api/Era/Internal/Eon/BabbageEraOnwards.hs Unifies Dijkstra-era placeholder error string in constraints helper.
cardano-api/src/Cardano/Api/Era/Internal/Eon/AllegraEraOnwards.hs Unifies Dijkstra-era placeholder error string in constraints helper.
cardano-api/src/Cardano/Api/Era/Internal/Case.hs Unifies Dijkstra-era placeholder error strings in era case combinators.
cardano-api/src/Cardano/Api/Certificate/Internal.hs Unifies Dijkstra-era placeholder error strings in anchor extraction logic.
.changes/20260420_cardano_api_dijkstra_placeholder_unification.yml Adds Herald fragment documenting the maintenance-only string/TODO unification.

All Dijkstra-era placeholder `error` messages and `TODO` comments in
`cardano-api/src/` now share the greppable token `TODO Dijkstra:`, so a
single `grep -r 'TODO Dijkstra'` locates every site that needs attention
when Dijkstra-era support lands. No behaviour change; this is a pure
message/comment unification across 30 sites in 16 files.

The runtime invariant assertion at `Tx/Internal/Body.hs:2858`
(`toScriptIndexDijkstra: unexpected DijkstraGuarding`) is intentionally
untouched — it is an impossible-case assertion, not a
pending-implementation stub.

The PlutusV4 placeholder at `Plutus/Internal/Script.hs:1302`
(`toShelleyScript: PlutusV4 not implemented yet.`) is tracked
separately as it blocks on ledger exposing a PlutusV4 constructor.
@Jimbo4350 Jimbo4350 force-pushed the jordan/dijkstra-placeholder-unification branch from 706a137 to 2020e05 Compare April 21, 2026 20:12
@Jimbo4350 Jimbo4350 added this pull request to the merge queue Apr 27, 2026
Merged via the queue into master with commit e8e80af Apr 27, 2026
33 checks passed
@Jimbo4350 Jimbo4350 deleted the jordan/dijkstra-placeholder-unification branch April 27, 2026 19:28
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.

3 participants