store scan details in the same format as server

[seanbudd]

Link to issue number:

Closes #19984

Summary of the issue:

Once an add-on is installed, VirusTotal scan results would be cached in a different format than what was served by the server.
This would cause issues in unpacking the scanned data.
This would cause scan results to fail to be loaded for installed add-ons.

Description of user facing changes:

Viewing scan results for installed add-ons should be fixed

Description of developer facing changes:

None

Description of development approach:

• Added a toDict method to the VirusTotalScanResults class in scanResults.py, enabling conversion of scan result objects to a dictionary format matching the original scan results structure.
• Updated the asdict method in AddonStoreModel to use toDict for VirusTotalScanResults fields, ensuring correct serialization.

Testing strategy:

Manual testing via add-on store manual test plan

Known issues with pull request:

none

Code Review Checklist:

• Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• API is compatible with existing add-ons.
• Security precautions taken.

[Copilot]

Pull request overview

Fixes installed add-on VirusTotal scan results failing to load by ensuring cached scan data is serialized in the same structure as the add-on store server payload.

Changes:

• Added VirusTotalScanResults.toDict() to serialize scan results in the server-compatible virusTotal -> last_analysis_stats shape.
• Updated _AddonGUIModel.asdict() to serialize VirusTotalScanResults using toDict() and store vtScanUrl alongside it.
• Modernized a few type hints/imports in addon.py while updating the license header.

Reviewed changes

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

File	Description
source/addonStore/models/scanResults.py	Adds toDict() for server-compatible serialization of VirusTotal scan results.
source/addonStore/models/addon.py	Updates dataclass serialization to use VirusTotalScanResults.toDict() and persist vtScanUrl.

[CyrilleB79]

@seanbudd should the error have disappeared? I still have the same errors when opening the store on last beta ().
Or should I remove some files? If yes, which ones?

Also, have you considered #19984 (comment)? If yes and if you have chosen to keep the structure as is, can you provide a reason? Storing a one million char in a json data is quite unusual (and freezing NVDA...)

[seanbudd]

@seanbudd should the error have disappeared? I still have the same errors when opening the store on last beta ().
Or should I remove some files? If yes, which ones?

The error will remain until the relevant add-on file e,g. source\userConfig\addons\addonId.json is updated/removed.
If the add-on is updated, this will be fixed automatically. removing the file will make it treated as an external add-on, not from the add-on store. The impact in the mean time, is users who have tested the beta/alpha will not be able to view the scan results for an installed add-on until this file is cleaned.

Also, have you considered #19984 (comment)? If yes and if you have chosen to keep the structure as is, can you provide a reason? Storing a one million char in a json data is quite unusual (and freezing NVDA...)

Changing something like this is beyond an emergency hot-fix during the translation freeze.
It has no real user impact and is only a developer facing change.
I would suggest opening an issue using the dev facing template.

[CyrilleB79]

The error will remain until the relevant add-on file e,g. source\userConfig\addons\addonId.json is updated/removed. If the add-on is updated, this will be fixed automatically. removing the file will make it treated as an external add-on, not from the add-on store. The impact in the mean time, is users who have tested the beta/alpha will not be able to view the scan results for an install add-on until this file is cleaned.

OK, so deinstalling/uninstalling the add-ons will be enough. Thanks.
Maybe a message targeting beta testers would be useful, so that they do not report errors now or later when they run rc, just because they will have run the beta before. If you do not do it yourself, I'll send one on the NVDA users mailing list.

Also, have you considered #19984 (comment)? If yes and if you have chosen to keep the structure as is, can you provide a reason? Storing a one million char in a json data is quite unusual (and freezing NVDA...)

Changing something like this is beyond an emergency hot-fix during the translation freeze. It has no real user impact and is only a developer facing change. I would suggest opening an issue using the dev facing template.

OK. So it's unrelated to this issue/fix? I'll open an issue if it's really problematic in the future.

I had provided a feedback on this point in case there was a broader impact than only for devs. Since it has an impact on the API, it may have had better to fix this before this goes in the final 2026.1 API version. It's worth noting that this had been introduced very late in the beta dev cycle (around beta6); that's why we end up now with something that cannot be changed anymore due to freeze (except if the freeze is extended).

[seanbudd]

I don't think it's worth an email as there is no significant impact. I don't think the scan details being missing for add-ons already installed is high impact - the add-on is already installed, the risk has already been taken.
We can always point reported issues to this ticket, but to me, sending out an email seems like unnecessary noise.

The comments regarding how the main data is stored as a string is unrelated to the issue that was fixed in the beta.
I don't think it's an issue other than the readability when manually checking the file. Could you explain what you mean bu this being introduced late in the beta cycle? I believe the virus total scanning stuff was released well before the beta, and the way data was stored in the "data" key has always been like that since add-on store release.
The issue is less about translation freeze, and more about taking unnecessary risk for 0 user impact.

[CyrilleB79]

We can always point reported issues to this ticket, but to me, sending out an email seems like unnecessary noise.

OK. In case we have too many reports or messages about these errors in the future, we can still send a message at that time if needed.

Could you explain what you mean bu this being introduced late in the beta cycle? I believe the virus total scanning stuff was released well before the beta, and the way data was stored in the "data" key has always been like that since add-on store release.

What I had in mind when writing "introduced late" was the pickle to json conversion (#19564). There is no specific usage regarding what is stored in pickle files. On the opposite, it's quite uncommon, even if not forbidden, to have 1 million char strings stored in a json field. One of the advantages of json is that they are easily readable in an editor; that's not the case for this string.

From user point of view, there is no change though. So I acknowledge that it's now too late for introducing any 0-user impact modifications now.

[seanbudd]

#19564 affected the enabled/disabled state of add-ons, it didn't change how we stored this add-on data from the add-on store, which was always in this JSON form.

[CyrilleB79]

#19564 affected the enabled/disabled state of add-ons, it didn't change how we stored this add-on data from the add-on store, which was always in this JSON form.

Oh so the issue is not new, but with no user impact, and I had never encountered it before...
I'm very sorry for all this confusion and for making you lose your time.

[seanbudd]

That's okay - it helped with investigating and I agree it would be nice to fix. It's not very readable for devs now