Skip to content

fix(ci): ignore byllm media glue in jac check, like the rest of byllm - #9057

Open
MusabMahmoodh wants to merge 2 commits into
jaseci-labs:mainfrom
MusabMahmoodh:fix/jacignore-byllm-media
Open

fix(ci): ignore byllm media glue in jac check, like the rest of byllm#9057
MusabMahmoodh wants to merge 2 commits into
jaseci-labs:mainfrom
MusabMahmoodh:fix/jacignore-byllm-media

Conversation

@MusabMahmoodh

Copy link
Copy Markdown
Contributor

What this changes

Adds one line to .jacignore:

jac/jaclang/byllm/types.impl/media.impl.jac

Why

jac check reports 13 errors in that file:

error[E1030]: Type "ImageFile" has no attribute "format"
error[E1030]: Type "ImageFile" has no attribute "save"
error[E1053]: Cannot assign <Unknown> | str to parameter 'fmt' of type str | NoneType

They are false positives. byllm/_optdeps/pillow.jac try-imports the real Pillow and falls back to an empty stub:

try {
    import from PIL.Image { Image as PILImageCls, open as open_image }
    HAS_PILLOW = True;
} except ImportError {
    class PILImageCls {}
    def open_image(*args: object, **kwargs: object) -> object { ... }
    HAS_PILLOW = False;
}

So open_image resolves to PIL's open, returning ImageFile, only when Pillow is installed in the checking environment. ImageFile extends Image.Image, which declares format: str | None = None (class level) and def save, so both attributes do exist - verified in Pillow 12.2.0 and 12.3.0, identical in both. The checker just does not follow them through that inheritance.

Why it shows up as flake

The errors appear only when Pillow resolves, and the jac-check job restores a cached JIR analysis keyed partly by prefix. Whether the file gets re-analysed against an environment with Pillow installed therefore varies run to run, so jac-check flips red and green on branches that touched nothing near byllm. Locally, where Pillow is not resolvable to the jac runtime, the same file checks clean with 0 errors.

Why ignore rather than annotate

Every sibling in this glue is already ignored - all of llm.impl/*, telemetry*, and types.jac itself - under a block whose comment reads "byllm - litellm/LLM-provider glue". This file is the same category of third-party glue and looks like a plain omission: types.jac is listed but its impl is not.

Annotating around a checker limitation would put noise in the source to work around a bug that is not in this file. The underlying checker gap - class-level attribute annotations not being found through a base class in a third-party package - is worth its own issue, and this PR does not fix it.

Validation

jac check jac/jaclang/byllm/types.impl/media.impl.jac locally: 0 errors, warnings only, at both 3ad35097 and current main - which is what established the errors are environment-dependent rather than a regression in the file.

The ignore mechanism is the one already used by the nine sibling entries: the jac-check job splices .jacignore into jac check . --ignore $IGNORE_ARGS.

`jac check` reports 13 errors in `byllm/types.impl/media.impl.jac`, all of
the form:

    error[E1030]: Type "ImageFile" has no attribute "format"
    error[E1030]: Type "ImageFile" has no attribute "save"

They are false positives. `_optdeps/pillow.jac` try-imports the real
`PIL.Image.Image` and falls back to an empty stub class when Pillow is
absent, so `open_image` resolves to PIL's `open` returning `ImageFile` only
when Pillow is installed in the checking environment. `ImageFile` extends
`Image.Image`, which declares `format: str | None = None` and `def save`,
so both attributes exist; the checker does not follow them through that
inheritance.

Because the errors appear only when Pillow resolves, and the job restores a
cached JIR analysis, jac-check flips red and green on unrelated branches.

Every sibling in this glue is already ignored, `types.jac` included, under a
block whose comment names exactly this category. Its impl was the omission.
MusabMahmoodh added a commit to MusabMahmoodh/jaseci that referenced this pull request Sep 8, 2026
Duplicates the single line in jaseci-labs#9057. jac-check on this branch fails with 13
false positives in byllm/types.impl/media.impl.jac that have nothing to do
with these changes; jaseci-labs#9057 explains why they are false and why the file
belongs with the rest of the byllm glue.

Drop this commit once jaseci-labs#9057 lands. Git collapses the identical line.

@christianwilkins christianwilkins left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed 919d632e816c862155087bf193f640803865d471 against its base, including the optional Pillow adapter and the CI command that reads .jacignore.

The added entry is path-specific, points to the existing implementation, and appears exactly once in the ignore arguments. No runtime code changes or blocking code findings. Local checks: target/entry validation and git diff --check passed. No fixes were made during this review.

One nonblocking wording correction: the release fragment should say the repository's CI jac check gate excludes this file. This patch changes the explicit ignore list supplied by the workflow; it does not repair inherited Pillow attribute resolution.

Current-head CI run 34191833660 succeeded and pre-commit.ci is green, but jac-check itself was skipped. I did not run a Pillow-enabled reproduction locally, so neither this review nor that green aggregate demonstrates the environment-dependent checker failure clearing. A run of the affected gate with Pillow available would close that validation gap. Status: no blocking code finding; the affected checker execution remains unverified.

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