Skip to content

fix: allow abbreviation on sorted mismatch maps - #241

Open
felipeofdev-ai wants to merge 1 commit into
nubank:masterfrom
felipeofdev-ai:fix/234-ellision-sorted-map
Open

felipeofdev-ai wants to merge 1 commit into
nubank:masterfrom
felipeofdev-ai:fix/234-ellision-sorted-map

Conversation

@felipeofdev-ai

Copy link
Copy Markdown

Summary

  • Convert sorted maps to hash-maps before associating EllisionMarker keys during abbreviation.
  • Add a unit test that previously would ClassCastException on PersistentTreeMap.

Why

with-ellision-marker used assoc with a non-Comparable marker key. Sorted mismatch maps require comparable keys, which surfaces as ClassCastException when *use-abbreviation* is enabled (issue #234).

Fixes #234

Test plan

  • matcher-combinators.printer-test/with-ellision-marker-sorted-map
  • Run printer / full test suite in CI

— Felipe Fernandes · Systems & Agentic AI Engineer
https://github.com/felipeofdev-ai · https://felipeofdev-ai.github.io/

with-ellision-marker assoc'd EllisionMarker into the mismatch map.
Sorted maps require Comparable keys, which caused ClassCastException
when *use-abbreviation* abbreviated a PersistentTreeMap.

Convert sorted maps to hash-maps before inserting ellision markers.

Fixes nubank#234

Signed-off-by: Felipe Fernandes <felipe.of.dev@gmail.com>
@felipeofdev-ai

Copy link
Copy Markdown
Author

Friendly bump: CI shows Nu workflow processing errors on pull_request (dashboard.nu.workflows.dev) rather than a failing project test. Happy to rebase or adjust if maintainers prefer a different approach for #234.

— Felipe Fernandes · https://github.com/felipeofdev-ai

@philomates

Copy link
Copy Markdown
Collaborator

Hi @felipeofdev-ai,
thanks for your interest in the library, I hope you've enjoyed using it so far.

I'm a co-maintainer here but haven't worked at Nubank for over 5 years. I have a personal policy were I don't review work done mainly by LLM-tools (elaborated slightly here). You might have luck with someone at Nubank who takes interest in the changes suggested here and in the 5 other pull requests you opened yesterday. That said, it has been a few years since anyone inside of Nubank has offered code reviews on pull requests here.

If you are indeed coming across these issues with the library and want to see them resolved, feel free to open an issue and we can talk over the trade-offs of how to go about resolving them, or I'm happy to offer support if you want to try to get familiar with the library and do it yourself (in such a case, I would ask you not to use LLM-tools while we collaborate). And of course, if you open a pull request with code that isn't largely written using LLM tools, I'm happy to review those.
Cheers, Phillip

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.

matcher-combinators.config/*use-abbreviation* throws error when actual doesn't match

2 participants