Skip to content

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Sep 26, 2025

PR Type

Enhancement


Description

  • Simplify markdown test report table

  • Remove failed column and zero-pass rows

  • Update formatting and headers accordingly


Diagram Walkthrough

flowchart LR
  A["helpers.report_to_markdown_table"] -- "drop Failed column" --> B["headers updated"]
  A -- "skip rows with 0 passed" --> C["filtered output"]
  A -- "format rows" --> D["| Test Type | Passed ✅ |"]
Loading

File Walkthrough

Relevant files
Enhancement
helpers.py
Report table shows only passed counts                                       

codeflash/lsp/helpers.py

  • Remove "Failed ❌" column from table header.
  • Skip rendering rows when passed equals zero.
  • Stop reading/using "failed" count from report.
  • Adjust row formatting to include only test type and passed.
+4/-4     

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


saga4 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Skipping rows when passed == 0 may unintentionally hide test types that had failures but no passes if upstream callers still supply a failed count or expect visibility of problem areas after removing the Failed column.

passed = report[test_type]["passed"]
# failed = report[test_type]["failed"]
if passed == 0:
    continue
Formatting

The header separator row has two columns; ensure column widths match the longest content to avoid misalignment in strict Markdown renderers. Consider matching dash counts to header text for readability.

lines = ["| Test Type | Passed ✅ |", "|-----------|--------|"]
for test_type in TestType:

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix Markdown table alignment

Ensure the header and separator column counts match the data rows to avoid malformed
Markdown tables. The separator should have the same number of columns as the header.

codeflash/lsp/helpers.py [32]

-lines = ["| Test Type | Passed ✅ |", "|-----------|--------|"]
+lines = ["| Test Type | Passed ✅ |", "|-----------|-----------|"]
Suggestion importance[1-10]: 6

__

Why: The header has two columns, so the separator should also have two columns; changing "--------" to "-----------" makes column widths consistent in Markdown tables.

Low
General
Preserve zero-pass result rows

Avoid filtering out rows solely based on passed count; this may hide test types with
all failures and produce incomplete reports. Keep the row whenever any result exists
for the test type.

codeflash/lsp/helpers.py [38-39]

-if passed == 0:
+if report[test_type].get("passed", 0) == 0 and report[test_type].get("failed", 0) == 0:
     continue
Suggestion importance[1-10]: 3

__

Why: The suggestion conflicts with the PR’s intentional change to omit failed counts entirely; reintroducing failed-based filtering is inconsistent with the new table format and may be inappropriate.

Low

@Saga4 Saga4 requested a review from mohammedahmed18 October 1, 2025 02:55
@Saga4 Saga4 changed the title [WIP] logger msg changes in VSC Logger msg changes in VSC Oct 1, 2025
@Saga4 Saga4 merged commit 649f04a into main Oct 6, 2025
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants