Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Summary of Changes

Hello @devs6186, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the IDA Explorer plugin where attempting to sort feature locations with mixed address types would result in a TypeError. The fix introduces a custom sort key to prevent incompatible type comparisons, ensuring robust handling of diverse address representations and improving the stability of feature rendering.

Highlights

• Fixed TypeError in IDA Explorer: Resolved a TypeError that occurred when sorting feature locations containing a mix of integer-based addresses (e.g., AbsoluteVirtualAddress) and _NoAddress types within the IDA Pro plugin.

New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

I have read the CLA Document and I hereby sign the CLA

Thanks for the review! Appreciate the confirmation that the sort key approach is clean.

this looks fine for ida-explorer, but i wonder if there are other places in the code with this pattern?

or, even better, could we fix the comparison routines for all addresses directly? so we can just do sorted(locations) and it works as intended?

this looks fine for ida-explorer, but i wonder if there are other places in the code with this pattern?

or, even better, could we fix the comparison routines for all addresses directly? so we can just do sorted(locations) and it works as intended?

Thanks a lot for the review!

_1. Other places with this pattern-:_
Yes -- the same pattern also appears in capa/ida/plugin/view.py.
Line 1054 uses sorted(features.items(), key=lambda k: sorted(k[1])), where sorted(k[1]) operates on lists of addresses that may include both AbsoluteVirtualAddress and _NoAddress.

Line 1077 does for addr in sorted(addrs), which can hit the same mixed-type comparison issue when a feature has no address (e.g., line 1085 constructs _NoAddress()).

So the same TypeError is possible there as well.

The only other sorted(locations) I found is in capa/render/vverbose.py (line 75), but that path uses frozen addresses (frz.Address). Those already implement a proper lt in capa/features/freeze, so they shouldn't be affected.

2. Fixing comparison at the source
I agree that fixing this in capa/features/address.py is the cleaner solution.
Right now, the failure happens because:
AbsoluteVirtualAddress subclasses int, so ava < noaddr delegates to int.lt, which returns NotImplemented.
Python then attempts the reflected operation _NoAddress.gt(ava).
Since _NoAddress doesn't define gt, that also resolves to NotImplemented, and Python raises a TypeError.
One straightforward fix would be to implement gt on _NoAddress, for example returning other is not self, effectively treating "no address" as greater than any real address (so it sorts last). With that in place, sorted(locations) would work consistently everywhere without needing per-call-site sort keys.

A Sample Snippet for your Reference before I Implement-

1. To Add gt to _NoAddress (so "no address" sorts after real addresses):

2. In capa/ida/plugin/model.py -- revert to a simple sort (no key):
   for location in sorted(locations):

3. In capa/ida/plugin/view.py -- keep using sorted(); no change needed once gt is in place:
   Line 1054: sorted(features.items(), key=lambda k: sorted(k[1]))
   Line 1077: for addr in sorted(addrs):
   Both will work as-is after the _NoAddress.gt change.

I'm happy to proceed in either of the following ways:
Expand this PR to add the same sort-key workaround in view.py (lines 1054 and 1077), and

Either in a follow-up PR (or in this one, if you prefer), add _NoAddress.gt in address.py and then remove the custom sort keys in both model.py and view.py, so we can just use sorted(locations) / sorted(addrs) directly.

Let me know whether you'd prefer the IDA-side workarounds first with the address fix in a separate PR, or if you'd like everything handled together in this one.

the gt solution sounds great, thanks for the careful study. let's update this PR with that strategy and use it consistently throughout.

Done. I've updated the PR to use the gt approach and keep it consistent:

capa/features/address.py -- Implemented gt on _NoAddress so that "no address" is greater than any real address and sorts last. That way sorted(locations) and sorted(addrs) work everywhere without special keys, and we avoid the mixed-type TypeError at the source.

capa/ida/plugin/model.py -- Dropped the custom sort key and now use plain sorted(locations).

capa/ida/plugin/view.py -- No edits. The existing sorted(k[1]) and sorted(addrs) (lines 1054 and 1077)
now behave correctly with mixed address types thanks to the address comparison fix.
I ran the engine and rules tests; all passed.