Skip to content

[Repo Assist] fix: make Frequency.isSubset truly generic (remove hardcoded float key type)#378

Draft
github-actions[bot] wants to merge 2 commits intodeveloperfrom
repo-assist/fix-frequency-issubset-generic-20260419-e59234ecd6f64431
Draft

[Repo Assist] fix: make Frequency.isSubset truly generic (remove hardcoded float key type)#378
github-actions[bot] wants to merge 2 commits intodeveloperfrom
repo-assist/fix-frequency-issubset-generic-20260419-e59234ecd6f64431

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist, an AI assistant.

Summary

Fixes a silent type-constraint bug in Frequency.isSubset.

Root cause: The inner recursive helper issubset had explicit type annotations list<float*int> and Map<float,int>. F# type inference propagated these constraints to the outer function, making isSubset accept only Map<float,int> — despite the outer signature appearing generic (Map<_,int>). Users calling isSubset on a string or int histogram (created via Frequency.createGeneric) would get a confusing type error.

Fix: Replace the hardcoded float type annotations in the inner helper with the generic type variable 'a, matching the intended semantics.

Before:

let isSubset (histA:Map<_,int>) (histB:Map<_,int>) =
    let rec issubset (histA:list<float*int>) (histB:Map<float,int>) = ...

After:

let isSubset (histA:Map<'a,int>) (histB:Map<'a,int>) =
    let rec issubset (histA:list<'a*int>) (histB:Map<'a,int>) = ...
```

## Test Status

6 new tests added covering float, string, int, and empty histogram cases:

```
Passed Distributions.Frequency.isSubset float keys - A is subset of B
Passed Distributions.Frequency.isSubset float keys - A is not subset of B
Passed Distributions.Frequency.isSubset string keys - A is subset of B
Passed Distributions.Frequency.isSubset string keys - A is not subset of B
Passed Distributions.Frequency.isSubset int keys - A is subset of B
Passed Distributions.Frequency.isSubset empty histA is always subset

✅ All 1205 tests passed (1199 pre-existing + 6 new).

Generated by 🌈 Repo Assist — an automated AI assistant. Workflow run

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

…y type)

The inner helper function issubset was typed as list<float*int> and
Map<float,int>, which forced F# type inference to constrain the outer
isSubset to Map<float,int> despite its signature appearing generic.
This meant isSubset could not be used with string, int, or any other
key type produced by Frequency.createGeneric.

Fix: replace explicit float type annotations in the helper with the
generic type variable 'a, making isSubset work for all comparable key
types.

Add 6 tests covering float, string, int, and empty histogram cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.99%. Comparing base (3533493) to head (e3f79d1).
⚠️ Report is 185 commits behind head on developer.

Additional details and impacted files
@@              Coverage Diff              @@
##           developer     #378      +/-   ##
=============================================
+ Coverage      47.61%   47.99%   +0.37%     
=============================================
  Files            152      135      -17     
  Lines          16765    12930    -3835     
  Branches        2253     1709     -544     
=============================================
- Hits            7983     6206    -1777     
+ Misses          8097     6205    -1892     
+ Partials         685      519     -166     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant