ci: run mypy across the codebase

[anakin87]

Related Issues

• happened that in feat: ChatGenerator protocol #9074 we changed some types in core classes
• since mypy is currently configured to run only on the changed python files of a PR, it did not catch errors
  

  haystack/.github/workflows/tests.yml

  Line 249 in dae8c7b

  hatch run test:types ${{ steps.files.outputs.all_changed_files }}

Proposed Changes:

• whenever at least one python file is changed, make mypy run across all the codebase

How did you test it?

CI; triggered a run by temporarily changing a python file (9b5457a) -> mypy run (failures in the logs are related to #9102)

In this case, mypy spent 1m22s (for comparison, in #9099, mypy ran on a single file and took 1m5s).

Notes for the reviewer

Perhaps we have not done this in the past because mypy is slow...
However, the lint job (including mypy) depends on unit tests and runs concurrently with integration tests (which are slower), so all in all I don't think this change would make people wait any longer.

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added unit tests and updated the docstrings
• I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
• I documented my code
• I ran pre-commit hooks and fixed any issue

[vblagoje]

Interesting that the difference is so small

[coveralls]

Pull Request Test Coverage Report for Build 14042523294

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 90.131%

---

Totals
Change from base Build 14042052718:	0.0%
Covered Lines:	9854
Relevant Lines:	10933

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 14042532370

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 90.131%

---

Totals
Change from base Build 14042052718:	0.0%
Covered Lines:	9854
Relevant Lines:	10933

---

💛 - Coveralls

[davidsbatista]

In this case, mypy spent 1m22s (for comparison, in #9099, mypy ran on a single file and took 1m5s).

This seems to be a too-small difference between running mypy over the whole code base and a single file - are you sure no caching was being used?

I would say we should only run a type checker over the changed files and the over whole code base when, for instance, mypy is updated.

@vblagoje @julian-risch what do you think?

[anakin87]

This seems to be a too-small difference between running mypy over the whole code base and a single file - are you sure no caching was being used?

I would say we should only run a type checker over the changed files and the over whole code base when, for instance, mypy is updated.

I'd propose always running mypy across the entire codebase for two reasons:

• if we change something in a core class and then run mypy only on this class, we risk missing relevant errors (as it recently happened - see the fix in fix: improve component.output_types decorator type hinting to support run_async methods #9102)
• lint job (which includes mypy) currently takes < 2 minutes, while integration tests running at the same time take around 7 minutes. Even if running mypy on all files adds 30s–1m to the lint job, it would not be a bottleneck for contributors.

[davidsbatista]

I understand now in detail the issue here - we still should keep an eye for the time this takes, my concern is that this can become a bootleneck

[anakin87]

I understand now in detail the issue here - we still should keep an eye for the time this takes, my concern is that this can become a bootleneck

I will keep an eye on it.