Repository navigation
Count completed Agent jobs in the daily AI Credits guardrail #66302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
db87e18
7e79ef5
833f1fe
86450fe
520e8c9
2c392c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,25 @@ it("counts an empty detection accounting file as zero AIC", async () => { | |
| expect(global.core.info).toHaveBeenCalledWith(expect.stringContaining('"source":"detection/token_usage.jsonl"')); | ||
| }); | ||
|
|
||
| it("counts compiler-generated Agent, Detection and Evals jobs using their lowercase accounting paths", async () => { | ||
| const f = evaluate( | ||
| { | ||
| "agent/token_usage.jsonl": '{"aic":18.02604}', | ||
| "detection/token_usage.jsonl": '{"aic":30.08748}', | ||
| "evals/token_usage.jsonl": '{"aic":1}', | ||
| }, | ||
| [job("Agent"), job("Detection"), job("Evals")] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] The compiler's real generated evals job name is "Evaluations" (see 💡 Why this mattersThe test at line 115 ( Suggested fix: either special-case @copilot please address this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handled in |
||
| ); | ||
| await expect(f.result).resolves.toBeCloseTo(49.11352); | ||
| expect(f.client.listArtifacts).toHaveBeenCalledOnce(); | ||
| expect(global.core.info).not.toHaveBeenCalledWith(expect.stringContaining('"reason":"no_billable_jobs"')); | ||
| }); | ||
|
|
||
| it("treats differently cased names for the same component as ambiguous", async () => { | ||
| const f = evaluate({}, [job("Agent", { id: 1 }), job("agent", { id: 2 })]); | ||
| await expect(f.result).rejects.toThrow("Ambiguous daily AIC component jobs"); | ||
| }); | ||
|
|
||
| it("still requires accounting when detection/token_usage.jsonl is missing (not empty)", async () => { | ||
| const f = evaluate( | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix correctly lowercases GitHub's display names
Agent/Detectionto matchCOMPONENT_FILES, but the real compiledevalsjob display name isEvaluations, notEvals(confirmed across all 167 compiled.github/workflows/*.lock.yml— e.g.ab-testing-advisor.lock.ymlline 1913:name: Evaluations)."evaluations".toLowerCase()does not match theevalskey inCOMPONENT_FILES(actions/setup/js/daily_aic_component_coverage.cjs:9-12), so completedEvaluationsjobs will still be silently skipped byObject.hasOwn(COMPONENT_FILES, name)— the exact same class of bug this PR is fixing forAgent/Detection, just left unfixed forevals.The new regression test at
daily_aic_component_coverage.test.cjs:108usesjob("Evals")instead of the real display namejob("Evaluations"), which masks this gap (the fabricated name happens to lowercase toevalsand pass).Suggest adding an
evaluations→evalsalias (or mapping GitHub's actual display names explicitly) and updating the test to use the real compiled name"Evaluations".@copilot please address this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handled in
86450fe:Evaluationsnow maps toevalsfor component and artifact accounting, with regression coverage using the actual display name.