Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

Description

Standardizes OCaml reference comments to use single-line hyperlinks instead of multi-line format, improving readability and IDE integration.

Old format:

/// OCaml reference: src/lib/mina_base/transaction_status.ml L:9-51
/// Commit: 5da42ccd72e791f164d4d200cf1ce300262873b3
/// Last verified: 2025-10-08

New format:

/// OCaml: <https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/mina_base/transaction_status.ml#L9-L51>

Changes made:

  • Code references (49 total): Converted all multi-line references to hyperlinks in 8 files across ledger, core, mina-p2p-messages, and node modules
  • CI script: Updated .github/scripts/check-ocaml-refs.sh to exclusively validate hyperlink format only (no legacy support). Removed all legacy multi-line format support, simplifying the script from 367 to 243 lines (33% reduction)
  • Documentation: Updated website/docs/developers/ocaml-reference-tracking.md and CLAUDE.md with new format specification and examples
  • Label preservation: Maintained "Protocol specification:" label for documentation links in core/src/constants.rs (distinct from OCaml implementation references)

Note: Auto-generated file mina-p2p-messages/src/v2/generated.rs (299 references) uses markdown link format and remains unchanged.

Related Issue(s)

Issue tracking handled separately

Type of Change

  • Refactoring (no functional changes)
  • This change requires a documentation update

Testing

  • Verified format checks pass (make check-format, make check-md, make check-trailing-whitespace)
  • Confirmed all 49 references converted correctly with proper commit hashes and line ranges
  • Validated CI script logic for parsing URL format and extracting components
  • Verified script only validates hyperlink format (no legacy format support)
  • Confirmed "Protocol specification:" label preserved for documentation links

Changelog Entry

Changelog Summary:

  • Changed: Migrated OCaml reference tracking from multi-line to single-line hyperlink format for improved readability and IDE integration
Original prompt

This section details on the original issue you should resolve

<issue_title>Migrate OCaml reference tracking to single-line hyperlink format</issue_title>
<issue_description>## Summary

Following a team retro, we need to migrate all OCaml reference comments to use
single-line hyperlinks for better trackability and simpler format.

Current state

We have two different formats for OCaml references in the codebase:

1. Multi-line format (documented/CI-tracked)

/// OCaml reference: src/lib/mina_base/transaction_status.ml L:9-51
/// Commit: 5da42ccd72e791f164d4d200cf1ce300262873b3
/// Last verified: 2025-10-08

Found in:

  • ledger/src/scan_state/transaction_logic/mod.rs (~20+ references)
  • ledger/src/scan_state/transaction_logic/for_tests.rs
  • ledger/src/scan_state/transaction_logic/signed_command.rs
  • ledger/src/account/account.rs

2. Single-line hyperlink format (not tracked by CI)

// <https://github.com/MinaProtocol/mina/blob/d800da86a764d8d37ffb8964dd8d54d9f522b358/src/lib/mina_block/validation.ml#L369>

Or with markdown link syntax:

/// Location: [src/lib/mina_block/block.ml:8:4](https://github.com/MinaProtocol/mina/blob/1551e2faaa/src/lib/mina_block/block.ml#L8)

Found in:

  • core/src/block/prevalidate.rs
  • core/src/consensus.rs
  • core/src/constants.rs
  • mina-p2p-messages/src/string.rs
  • node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs
  • mina-p2p-messages/src/v2/generated.rs (auto-generated, many entries)

Proposed new format

Standardize on a single-line hyperlink format:

/// OCaml: <https://github.com/MinaProtocol/mina/blob/COMMIT_HASH/path/to/file.ml#L9-L51>

Benefits:

  • Single line is easier to read and maintain
  • Clickable in IDEs and GitHub
  • Commit hash embedded in URL preserves historical tracking
  • Line range visible in URL fragment

Tasks

Phase 1: Update existing multi-line format to hyperlinks

  • Convert all multi-line /// OCaml reference: comments to hyperlink format
  • Files to update:
    • ledger/src/scan_state/transaction_logic/mod.rs
    • ledger/src/scan_state/transaction_logic/for_tests.rs
    • ledger/src/scan_state/transaction_logic/signed_command.rs
    • ledger/src/account/account.rs

Phase 2: Standardize existing hyperlinks

  • Audit and standardize existing hyperlink references to use consistent format
  • Files to review:
    • core/src/block/prevalidate.rs
    • core/src/consensus.rs
    • core/src/constants.rs
    • mina-p2p-messages/src/string.rs
    • node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs
  • Decide how to handle mina-p2p-messages/src/v2/generated.rs (auto-generated)

Phase 3: Update documentation

  • Update website/docs/developers/ocaml-reference-tracking.md:
    • Document new hyperlink format
    • Update examples
    • Remove multi-line format documentation
  • Update CLAUDE.md if OCaml reference format is mentioned

Phase 4: Update CI

  • Update .github/scripts/check-ocaml-refs.sh:
    • Parse hyperlink format instead of multi-line format
    • Extract commit hash and path from URL
    • Validate file existence and line ranges
    • Support auto-update by replacing commit hash in URLs
  • Update .github/workflows/check-ocaml-refs.yaml if needed
  • Test CI locally with gh act

Implementation notes

  1. The hyperlink format should use angle brackets for automatic linking:
    <https://...> not just https://...

  2. Consider using a doc comment prefix like /// OCaml: to make references
    searchable

  3. For auto-generated files like generated.rs, coordinate with the code
    generator tool

References

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Migrate OCaml reference tracking to single-line hyperlink format Migrate OCaml reference tracking to single-line hyperlink format Dec 16, 2025
Copilot AI requested a review from dannywillems December 16, 2025 02:05
# Usage: ./.github/scripts/check-ocaml-refs.sh [--repo REPO_URL] [--branch BRANCH] [--update]
#
# Supports two formats:
# 1. New hyperlink format: /// OCaml: <https://github.com/MinaProtocol/mina/blob/COMMIT/path#L1-L10>
Copy link
Member

Choose a reason for hiding this comment

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

@copilot we don't want to support both formats. We want to support only the new hyperlink format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in d6c16fb. The CI script now only supports the hyperlink format. Removed all legacy multi-line format parsing and validation code, simplifying the script from 367 to 243 lines.

@github-actions
Copy link

github-actions bot commented Dec 16, 2025

OCaml Reference Validation Results

Repository: https://github.com/MinaProtocol/mina.git
Branch: compatible
Status: ❌ Validation failed

Click to see full validation output
Checking OCaml references against https://github.com/MinaProtocol/mina.git (branch: compatible)
Fetching current commit from compatible...
Current OCaml commit: e099ecd8d03b739b98c74a6f383504a759a9740a

Validating references...
========================
✓ VALID: crates/ledger/src/account/account.rs:1273 -> src/lib/mina_base/account.ml L:201-224
  ⚠ STALE COMMIT: fc6be4c58091c761f827c858229c2edf9519e941 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
❌ INVALID: crates/ledger/src/scan_state/transaction_logic/for_tests.rs:63
   Code at L:2285-2285 differs between commit 5da42ccd72e791f164d4d200cf1ce300262873b3 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/transaction_logic/mina_transaction_logic.ml#L2285-L2285
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2285-L2285
❌ INVALID: crates/ledger/src/scan_state/transaction_logic/for_tests.rs:67
   Code at L:2351-2356 differs between commit 5da42ccd72e791f164d4d200cf1ce300262873b3 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/transaction_logic/mina_transaction_logic.ml#L2351-L2356
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2351-L2356
❌ INVALID: crates/ledger/src/scan_state/transaction_logic/for_tests.rs:76
   Code at L:2407-2407 differs between commit 5da42ccd72e791f164d4d200cf1ce300262873b3 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/transaction_logic/mina_transaction_logic.ml#L2407-L2407
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2407-L2407
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:118 -> src/lib/mina_base/transaction_status.ml L:9-51
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:234 -> src/lib/mina_base/transaction_status.ml L:452-454
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:250 -> src/lib/mina_base/with_status.ml L:6-10
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:320 -> src/lib/mina_base/fee_transfer.ml L:76-80
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:345 -> src/lib/mina_base/fee_transfer.ml L:68-69
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:420 -> src/lib/mina_base/coinbase.ml L:17-21
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:1049 -> src/lib/transaction/transaction.ml L:8-11
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:17 -> src/lib/mina_base/signed_command_payload.ml L:34-48
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:43 -> src/lib/mina_base/stake_delegation.ml L:11-13
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:70 -> src/lib/mina_base/signed_command_payload.ml L:179-181
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:81 -> src/lib/mina_base/signed_command_payload.ml L:239-243
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
✓ VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:112 -> src/lib/mina_base/signed_command_payload.ml L:352-362
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e099ecd8d03b739b98c74a6f383504a759a9740a)
❌ INVALID: crates/p2p-messages/src/string.rs:14
   Code at L:140-140 differs between commit c0c9d702b8cba34a603a28001c293ca462b1dfec and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/c0c9d702b8cba34a603a28001c293ca462b1dfec/src/lib/mina_base/zkapp_account.ml#L140-L140
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/mina_base/zkapp_account.ml#L140-L140
✓ VALID: crates/p2p-messages/src/string.rs:16 -> src/lib/mina_base/account.ml L:92-92
  ⚠ STALE COMMIT: c0c9d702b8cba34a603a28001c293ca462b1dfec (current: e099ecd8d03b739b98c74a6f383504a759a9740a)

Summary
=======
Total references found: 18
Valid references: 14
Invalid references: 4
Stale commits: 14

❌ Validation failed: 4 invalid reference(s) found

@github-actions
Copy link

github-actions bot commented Dec 16, 2025

⚠️ Code Reference Verification Failed

The documentation contains code references that do not match the current state of the codebase on the develop branch.

Issues Found

  • website/docs/developers/referencing-code-in-documentation.md:36 - File not found: ledger/src/scan_state/transaction_logic/valid.rs

Action Required

The code referenced in the documentation must be merged to develop before documentation can be added/modified.

Please follow this workflow:

  1. Merge the code changes to develop first (this PR or a separate code PR)
  2. Create a follow-up PR with the documentation updates that reference the merged code
  3. The verification will pass once the code is available on develop

See the documentation guidelines for more information about the two-PR workflow.

/// Related OCaml implementation: <https://github.com/MinaProtocol/mina/tree/compatible/src/config>
/// Protocol specification: <https://github.com/MinaProtocol/mina/blob/compatible/docs/specs/types_and_structures/serialized_key.md>
/// OCaml: <https://github.com/MinaProtocol/mina/tree/compatible/src/config>
/// OCaml: <https://github.com/MinaProtocol/mina/blob/compatible/docs/specs/types_and_structures/serialized_key.md>
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Keep "Protocol specification" here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored in 2356e5a. Changed back to "Protocol specification:" since this is a documentation link, not an OCaml implementation reference.

@dannywillems
Copy link
Member

@copilot rebase

Copy link
Contributor Author

Copilot AI commented Dec 16, 2025

@copilot rebase

I'm unable to perform git rebase operations directly. Rebasing requires executing git commands that are outside my available capabilities. You'll need to perform the rebase manually or have someone with git access to the repository handle it.

@dannywillems dannywillems force-pushed the copilot/migrate-ocaml-references branch from 2356e5a to c1b7f75 Compare December 16, 2025 23:31
@dannywillems dannywillems marked this pull request as ready for review December 16, 2025 23:33
Copilot AI and others added 3 commits December 16, 2025 22:49
Converted 43 OCaml references from multi-line format to single-line hyperlink format across 4 files:
- ledger/src/scan_state/transaction_logic/mod.rs (24 refs)
- ledger/src/scan_state/transaction_logic/signed_command.rs (15 refs)
- ledger/src/scan_state/transaction_logic/for_tests.rs (3 refs)
- ledger/src/account/account.rs (1 ref)

Old format:
/// OCaml reference: src/lib/mina_base/transaction_status.ml L:9-51
/// Commit: 5da42ccd72e791f164d4d200cf1ce300262873b3
/// Last verified: 2025-10-08

New format:
/// OCaml: <https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/mina_base/transaction_status.ml#L9-L51>

Co-authored-by: dannywillems <[email protected]>
Standardized 6 OCaml references in 4 files to use consistent format:
- core/src/block/prevalidate.rs (2 refs)
- core/src/constants.rs (2 refs)
- mina-p2p-messages/src/string.rs (2 refs)
- node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs (1 ref)

All references now use: /// OCaml: <URL>

Note: mina-p2p-messages/src/v2/generated.rs is auto-generated and uses
markdown link format. This will be handled by updating the code generator
in a separate effort.

Co-authored-by: dannywillems <[email protected]>
Updated documentation to reflect the new single-line hyperlink format:
- website/docs/developers/ocaml-reference-tracking.md: Complete rewrite of format specification and examples
- CLAUDE.md: Updated example to use hyperlink format

The new format is simpler and more maintainable:
/// OCaml: <https://github.com/MinaProtocol/mina/blob/COMMIT/path#L1-L10>

Co-authored-by: dannywillems <[email protected]>
Copilot AI and others added 4 commits December 16, 2025 22:49
Updated .github/scripts/check-ocaml-refs.sh to parse and validate both:
- New hyperlink format: /// OCaml: <https://github.com/MinaProtocol/mina/blob/COMMIT/path#L1-L10>
- Legacy multi-line format (for backward compatibility)

The script now:
- Extracts commit hash, file path, and line ranges from URL fragments
- Validates file existence and line ranges for both formats
- Supports auto-update mode to refresh commit hashes in URLs
- Maintains backward compatibility during the transition period

Updated documentation to reflect that the script now fully supports the new format.

Co-authored-by: dannywillems <[email protected]>
Applied markdown formatting to documentation.
All format checks pass successfully.

Co-authored-by: dannywillems <[email protected]>
Updated .github/scripts/check-ocaml-refs.sh to only support the new hyperlink format.
Removed all legacy multi-line format parsing and validation code.

Changes:
- Removed legacy format detection (lines 69, 190-290, 333-350)
- Simplified script from 367 to 243 lines
- Updated header comment to reflect hyperlink-only support
- Updated documentation to remove backward compatibility mention

The script now exclusively validates:
/// OCaml: <https://github.com/MinaProtocol/mina/blob/COMMIT/path#L1-L10>

Co-authored-by: dannywillems <[email protected]>
Changed line 91 in core/src/constants.rs back to "Protocol specification:"
instead of "OCaml:" since this is a link to protocol documentation,
not OCaml implementation code.

Co-authored-by: dannywillems <[email protected]>
@dannywillems dannywillems force-pushed the copilot/migrate-ocaml-references branch from c1b7f75 to 82fc86c Compare December 17, 2025 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Migrate OCaml reference tracking to single-line hyperlink format

2 participants