Skip to content

Add Copilot instructions to suppress false positives on shared qualified import aliases#1132

Merged
carbolymer merged 1 commit intomasterfrom
copilot/sub-pr-1128-again
Mar 13, 2026
Merged

Add Copilot instructions to suppress false positives on shared qualified import aliases#1132
carbolymer merged 1 commit intomasterfrom
copilot/sub-pr-1128-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 12, 2026

Changelog

- description: |
    Add .github/copilot-instructions.md with Haskell-specific guidance to prevent
    GitHub Copilot from incorrectly flagging valid patterns (e.g. multiple modules
    sharing the same qualified import alias) as compile errors in future PR reviews.
  type:
  - maintenance
  projects:
  - cardano-api

Context

GitHub Copilot incorrectly flagged multiple modules sharing the same qualified import alias as a compile error (e.g., both Cardano.Api.Experimental and Cardano.Api.Experimental.Tx imported as Exp). This is valid GHC Haskell — name resolution happens at the use site, and ambiguity only arises if both modules export the same identifier used without disambiguation.

Addresses feedback from #1128.

How to trust this PR

The only file changed is .github/copilot-instructions.md. Verify its content explains the valid Haskell qualified-alias pattern correctly.

Checklist

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

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] [WIP] Address feedback on moving calcMinFeeRecursive tests to dedicated module Add Copilot instructions to suppress false positives on shared qualified import aliases Mar 12, 2026
@carbolymer carbolymer changed the base branch from jordan/1121-move-calc-min-fee-recursive-tests to master March 12, 2026 11:49
@carbolymer
Copy link
Copy Markdown
Contributor

@copilot see the PR description template in the repo, change the PR template to match it

@carbolymer carbolymer marked this pull request as ready for review March 12, 2026 11:50
Copilot AI review requested due to automatic review settings March 12, 2026 11:50
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot see the PR description template in the repo, change the PR template to match it

Updated the PR description to follow the repository template format (Changelog YAML, Context, How to trust this PR, Checklist).

@carbolymer
Copy link
Copy Markdown
Contributor

@copilot Rebase just your commit with a change onto master, then push with force to update this branch.

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

Adds repository-level GitHub Copilot guidance for a Haskell-specific import pattern, and refactors/extends the test suite by moving calcMinFeeRecursive properties into a dedicated module.

Changes:

  • Add .github/copilot-instructions.md documenting that shared qualified import aliases are valid in GHC Haskell and shouldn’t be flagged by Copilot.
  • Extract calcMinFeeRecursive property tests into new Test.Cardano.Api.Experimental.Fee and wire it into the test runner and cabal test-suite.
  • Export exampleProtocolParams from Test.Cardano.Api.Experimental so it can be reused by the new fee test module.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/copilot-instructions.md Adds Copilot instructions about shared qualified import aliases in Haskell.
cardano-api/test/cardano-api-test/cardano-api-test.hs Registers and runs the new Experimental.Fee test group.
cardano-api/test/cardano-api-test/Test/Cardano/Api/Experimental/Fee.hs New property-test module covering calcMinFeeRecursive scenarios.
cardano-api/test/cardano-api-test/Test/Cardano/Api/Experimental.hs Removes inlined fee properties and exports exampleProtocolParams for reuse.
cardano-api/cardano-api.cabal Adds the new fee test module to the cardano-api-test suite other-modules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/copilot-instructions.md
Comment on lines +115 to +122
fundingTxOut =
Exp.obtainCommonConstraints era $
L.mkBasicTxOut addr (L.MaryValue fundingCoin mempty)
utxo = L.UTxO $ Map.singleton ledgerTxIn fundingTxOut
sendTxOut =
Exp.obtainCommonConstraints era $
Exp.TxOut $
Ledger.mkBasicTxOut addr (L.MaryValue sendCoin mempty)
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Within this module, mkBasicTxOut is called sometimes via L.mkBasicTxOut and sometimes via Ledger.mkBasicTxOut, even though both qualifiers refer to Cardano.Api.Ledger. Using a single qualifier consistently here (as in Test.Cardano.Api.Experimental) would reduce confusion, especially given L is also shared by multiple Ledger modules in these tests.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot Rebase just your commit with a change onto master, then push with force to update this branch.

I can perform the rebase locally, but I'm unable to force-push — my tooling only supports regular git push, which would be rejected after a history rewrite. Could you run the following to clean this up?

git fetch origin master
git checkout copilot/sub-pr-1128-again
git rebase --onto origin/master de666e0~1 HEAD
git push --force origin copilot/sub-pr-1128-again

This will drop the de666e0 (Jordan Millar's) and 0c8938a (empty "Initial plan") commits, keeping only the .github/copilot-instructions.md change on top of master.

…ll qualified import aliases

Co-authored-by: carbolymer <228866+carbolymer@users.noreply.github.com>
@carbolymer carbolymer force-pushed the copilot/sub-pr-1128-again branch from 31c9bff to 828d721 Compare March 12, 2026 11:56
@carbolymer carbolymer added this pull request to the merge queue Mar 13, 2026
Merged via the queue into master with commit 2aa5ce7 Mar 13, 2026
32 checks passed
@carbolymer carbolymer deleted the copilot/sub-pr-1128-again branch March 13, 2026 09:35
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.

4 participants