Skip to content

Accept 100M/100K token notation for templatable token-limit fields#36928

Merged
pelikhan merged 7 commits into
mainfrom
copilot/extend-templatable-integer-type
Jun 4, 2026
Merged

Accept 100M/100K token notation for templatable token-limit fields#36928
pelikhan merged 7 commits into
mainfrom
copilot/extend-templatable-integer-type

Conversation

Copilot AI commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

max-effective-tokens and max-daily-effective-tokens were still validated as generic templatable_integer, so values like 100M/100K failed schema validation (as seen in the compile failure). This change aligns those fields with the token-limit schema that already supports K/M suffix notation, including lowercase variants.

  • Schema alignment

    • Rewired top-level schema fields:
      • max-effective-tokens#/$defs/max_effective_tokens_limit
      • max-daily-effective-tokens#/$defs/max_daily_effective_tokens_limit
    • This enables string token notation (K/M, case-insensitive) and preserves expression support.
  • Parser schema tests

    • Converted prior suffix rejection expectations into acceptance expectations.
    • Added explicit mixed-case coverage across both fields (100M, 100k, 100K, 100m patterns).
    • Updated zero/negative expectation cases to match these defs (0 invalid; -1 sentinel behavior where defined).
  • Example

    max-effective-tokens: 100K
    max-daily-effective-tokens: 100m

Copilot AI and others added 5 commits June 4, 2026 15:00
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Support K/M suffix token limits for templatable token fields Accept 100M/100K token notation for templatable token-limit fields Jun 4, 2026
Copilot AI requested a review from pelikhan June 4, 2026 15:06
},
"max-daily-effective-tokens": {
"$ref": "#/$defs/templatable_integer",
"$ref": "#/$defs/max_daily_effective_tokens_limit",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot refactor and reuse same def

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Refactored to reuse a shared schema def (positive_effective_tokens_limit) across both token-limit fields in commit 4f94739.

Copilot AI and others added 2 commits June 4, 2026 15:18
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan June 4, 2026 15:20
@pelikhan pelikhan marked this pull request as ready for review June 4, 2026 15:59
Copilot AI review requested due to automatic review settings June 4, 2026 15:59
@pelikhan pelikhan merged commit fb49656 into main Jun 4, 2026
1 check passed
@pelikhan pelikhan deleted the copilot/extend-templatable-integer-type branch June 4, 2026 16:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns the main workflow frontmatter schema so max-effective-tokens and max-daily-effective-tokens accept the same token-limit notation already supported elsewhere (e.g., 100K/100M, case-insensitive) while preserving GitHub Actions expression support.

Changes:

  • Rewired top-level schema properties to reference token-limit-specific $defs instead of templatable_integer.
  • Refactored/expanded $defs for effective-token limits to clearly separate “positive limit” vs “disabled via negative / -1”.
  • Updated parser schema validation tests to expect suffix strings and -1 sentinel values to pass, and 0 to fail.
Show a summary per file
File Description
pkg/parser/schemas/main_workflow_schema.json Repoints max-effective-tokens / max-daily-effective-tokens to token-limit defs and introduces a shared “positive effective tokens limit” helper to allow K/M suffix + expressions.
pkg/parser/schema_test.go Updates schema validation tests for 0, -1, and K/M-suffixed string acceptance (including lowercase support).
.github/skills/agentic-workflows/SKILL.md Adds additional referenced docs to the skill’s “load these files” list.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment thread pkg/parser/schema_test.go
Comment on lines +556 to +560
validFrontmatter := map[string]any{
"on": "push",
"max-effective-tokens": "100k",
"max-daily-effective-tokens": "100M",
}
Comment thread pkg/parser/schema_test.go
}

func TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_MaxDailyEffectiveTokensNegativeInvalid(t *testing.T) {
func TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_MaxDailyEffectiveTokensNegativeAllowed(t *testing.T) {
Comment thread pkg/parser/schema_test.go
t.Fatal("expected negative max-daily-effective-tokens to fail schema validation")
err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(validFrontmatter, "/tmp/gh-aw/max-daily-effective-tokens-negative-test.md")
if err != nil {
t.Fatalf("expected negative max-daily-effective-tokens to pass schema validation, got: %v", err)
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.

3 participants