Skip to content

feat: enable coverage check for js/ts#1525

Merged
mohammedahmed18 merged 3 commits intomainfrom
feat/enable-coverage-check-all-languages
Mar 17, 2026
Merged

feat: enable coverage check for js/ts#1525
mohammedahmed18 merged 3 commits intomainfrom
feat/enable-coverage-check-all-languages

Conversation

@mohammedahmed18
Copy link
Copy Markdown
Contributor

@mohammedahmed18 mohammedahmed18 commented Feb 18, 2026

No description provided.

Remove the Python-only gate on coverage checking so JS/TS optimizations
also require adequate test coverage before proceeding.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Feb 18, 2026

Claude finished @mohammedahmed18's task in 2m 58s —— View job


PR Review Summary

  • Triage PR scope
  • Run lint and type checks
  • Resolve stale review threads
  • Code review
  • Duplicate detection
  • Test coverage analysis

Prek Checks

All checks pass cleanly. The existing mypy error (Language not exported from codeflash.languages.base) is pre-existing and unrelated to this PR.


Code Review

The change is minimal and correct — it mirrors the exact pattern already used by PythonFunctionOptimizer to opt into coverage validation.

One stale comment to fix: function_optimizer.py:2716 still reads # Skip coverage check for non-Python languages (coverage not yet supported) — this is now incorrect since JS/TS coverage is enabled by this PR. Fix this →

No bugs, security issues, or breaking changes.


Duplicate Detection

No duplicates detected. Both PythonFunctionOptimizer and JavaScriptFunctionOptimizer override should_check_coverage() returning True — this is intentional protocol-based dispatch, not duplication (the base class defaults to False).


Test Coverage

No new tests added for should_check_coverage() on JavaScriptFunctionOptimizer. Given it's a single-line override that unlocks existing infrastructure, this is acceptable. The coverage infrastructure itself (JestCoverageUtils, coverage_critic) already has tests in tests/code_utils/test_coverage_utils.py.


Last updated: 2026-03-17
| Branch

@aseembits93
Copy link
Copy Markdown
Contributor

@claude why are some ci workflows failing?

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Feb 18, 2026

Claude finished @aseembits93's task in 2m 32s —— View job


CI Failure Analysis

  • Check CI status for this PR
  • Investigate failing workflows
  • Summarize findings

4 out of 19 workflows failed. All 4 failures are caused by the CodeFlash AI backend service (app.codeflash.ai) returning errors — not by the code changes in this PR. Here's the breakdown:

Workflow Root Cause
E2E - Tracer Replay Backend returned 500 - Internal server error for test generation and optimization requests. The final failure is an AssertionError: Failed to find coverage data for funcA because the optimization never completed successfully.
E2E - Async Backend returned 500 - Failed to generate valid async test code and later hit ReadTimeout (app.codeflash.ai port 443, read timeout=90s). All 3 retry attempts failed.
E2E - JS CommonJS Function Backend hit ReadTimeout on app.codeflash.ai during test generation for fibonacci. All 3 retry attempts failed with "NO TESTS GENERATED".
E2E - JS ESM Async Backend hit ReadTimeout on app.codeflash.ai during test generation for processItemsSequential. All 3 retry attempts failed with "NO TESTS GENERATED".

Summary: These are all infrastructure/backend flakiness issues — the CodeFlash API service was returning 500 errors and timing out during this CI run. None of the failures are related to the code change in this PR (removing the is_python() guard on coverage checking). The 15 other workflows (including Lint, Mypy, unit tests, Windows unit tests, and several other E2E tests) all passed.

These E2E tests are inherently flaky since they depend on an external AI service. A re-run should resolve them if the backend is healthy.


| Branch

aseembits93
aseembits93 previously approved these changes Feb 23, 2026
@mohammedahmed18 mohammedahmed18 changed the title feat: enable coverage check for all languages feat: enable coverage check for js/ts Mar 17, 2026
@aseembits93
Copy link
Copy Markdown
Contributor

can probably delete the function if it is only returning true @mohammedahmed18 ?

@aseembits93
Copy link
Copy Markdown
Contributor

i see you are doing some kind of inheritance

@mohammedahmed18
Copy link
Copy Markdown
Contributor Author

i see you are doing some kind of inheritance

yeah, kevin made a refactor so that each language has its own function optimizer inheriting the base class

@mohammedahmed18 mohammedahmed18 merged commit 6e867c9 into main Mar 17, 2026
28 checks passed
@mohammedahmed18 mohammedahmed18 deleted the feat/enable-coverage-check-all-languages branch March 17, 2026 13:09
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