Skip to content

Fix call to make_finding with wrong detail value type#188

Draft
RyanJarv wants to merge 2 commits intopeteromallet:mainfrom
RyanJarv:fix-make-finding-call
Draft

Fix call to make_finding with wrong detail value type#188
RyanJarv wants to merge 2 commits intopeteromallet:mainfrom
RyanJarv:fix-make-finding-call

Conversation

@RyanJarv
Copy link

@RyanJarv RyanJarv commented Mar 3, 2026

The make_finding call signature takes a dict for the detail arg not a string.

This comes up when running desloppify show ./dir:

...
Traceback (most recent call last):
  File "/home/parallels/desloppify/.venv/bin/desloppify", line 10, in <module>
    sys.exit(main())
             ~~~~^^
  File "/home/parallels/desloppify/desloppify/cli.py", line 147, in main
    handler(args)
    ~~~~~~~^^^^^^
  File "/home/parallels/desloppify/desloppify/app/commands/show/cmd.py", line 290, in cmd_show
    render_findings(
    ~~~~~~~~~~~~~~~^
        surfaced_matches,
        ^^^^^^^^^^^^^^^^^
    ...<8 lines>...
        budget_warning=budget_warning,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/home/parallels/desloppify/desloppify/app/commands/show/render.py", line 142, in render_findings
    _print_single_finding(finding, show_code=show_code)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/parallels/desloppify/desloppify/app/commands/show/render.py", line 59, in _print_single_finding
    detail_parts = format_detail(finding.get("detail", {}))
  File "/home/parallels/desloppify/desloppify/app/commands/show/formatting.py", line 38, in format_detail
    value = detail.get(key)
            ^^^^^^^^^^
AttributeError: 'str' object has no attribute 'get'

Follow-up to 385a7527f5df921622ae8987012ae94993e945b3.
Keep make_finding/show signature behavior unchanged while fixing the source of the mismatch: tree-sitter responsibility cohesion now emits dict detail, with tests asserting the new fields.
Comment on lines -63 to -64
if not isinstance(pattern, str) or not isinstance(hidden_by_detector, dict):
return 0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems there was only one call to make_finding with a string, so this fix was a bit simpler. The mypy config should catch calls with incorrect types to make_finding now.

Copy link
Author

@RyanJarv RyanJarv Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this isn't backwards compatible. Not sure if that matters for .desloppify/state-go.json?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest commit normalizes the old state now

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched this to a draft because I'm not particularly confident this is handled the right way. Besides that though, it works.

@RyanJarv RyanJarv marked this pull request as draft March 3, 2026 21:54
@peteromallet
Copy link
Owner

Thanks for catching this — really appreciate the bug report and fix. The detail-as-string crash in next/render.py has already been fixed on main (there's an isinstance(detail, str) guard at render.py:111 now). The show/formatting.py path still needs the guard though — I'll get that patched up on my end since I'm pushing a bunch of changes right now anyway. Thanks again!

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.

2 participants