Skip to content

#278 add upper version bounds for fbe and judges #278

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.

Sign up for GitHub

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

Open
VasilevNStas wants to merge 1 commit into
base: master
Choose a base branch
from
+52 −42
Open

#278 add upper version bounds for fbe and judges #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

source 'https://rubygems.org'

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Worth being explicit about what this file is before deciding the policy question. swarm-template is a GitHub template repository (is_template: true; the description reads "Use this repository as a template to create your own swarm of judges"), so every gem line below this one is copied verbatim into each generated swarm and then diverges independently. A constraint added here is not a decision for one repo, it is a default for every swarm created from this point forward, and those copies get no notification when the constraint becomes wrong. Relaxing it later means a PR in each downstream swarm individually. That asymmetry argues for the template shipping the loosest constraint that is still honest, and for reproducibility to be enforced by the lockfile rather than by a range.

Which is the more useful point: the reproducibility problem the description describes is already solved by Gemfile.lock, and the one real gap in that solution is not addressed by this PR. Nothing in the repo enables frozen mode -- there is no bundle config set frozen true, no --deployment, and .github/workflows/rake.yml uses ruby/setup-ruby with bundler-cache: true followed by a plain bundle install --no-color. In that setup, if Gemfile and Gemfile.lock disagree, Bundler silently re-resolves and rewrites the lock in the CI workspace instead of failing. A violated constraint is therefore not something CI can catch today, and the constraint being added here is not something CI will ever exercise.

If the goal really is "an accidental bundle update must not silently change what runs in production", the change that delivers it is BUNDLE_FROZEN: 'true' in the rake job env (or bundle config set --local frozen true committed to .bundle/config), so that any Gemfile/lock drift fails the build loudly. That is a smaller diff than this one and it covers all eleven gems instead of two.


gem 'fbe', '>0'
gem 'judges', '>0', require: false
gem 'fbe', '~>0'

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

~>0 expands to >= 0, < 1, so the only thing this excludes is a 1.0 release of fbe. fbe is at 0.49.1 (released 2026-07-17) and has been in the 0.x series for its entire life; there is no 1.0 on rubygems and no pre-release of one. As written this constraint is a no-op against every version that exists, and it stays a no-op right up until the day it isn't -- at which point it blocks an upgrade rather than making one safe.

The description gets the risk analysis right and then applies the wrong operator to it: "pre-1.0 gems where minor version bumps may introduce breaking changes". If that premise holds -- and it does; see line 77 of the lockfile in this same diff, where fbe moved its factbase requirement from ~> 0.11 to ~> 0.19 across 0.47.0 -> 0.48.5 -- then the transitions that need bounding are 0.48 -> 0.49, not 0.99 -> 1.0. ~>0 permits exactly the transitions the description warns about and blocks only the one it never discusses.

Expressing the stated intent in Ruby needs three segments: ~> 0.48.5 means >= 0.48.5, < 0.49, the pre-1.0 analogue of a caret bound. Note that ~> 0.48 would not do it -- that is >= 0.48, < 1.0, the same practical effect as what is written here. So the choice is a genuine one between ~> 0.48.5 (real gate, needs deliberate raising every minor) and leaving >0 and trusting the lockfile. ~>0 is the option that has the maintenance cost of a constraint without the protection of one.

gem 'judges', '~>0', require: false

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The case for bounding judges is stronger than for fbe, because judges is consumed as a CLI rather than as a library. entry.sh invokes judges update --summary --max-cycles=3 --no-log --option "id=${id}" --lib ... and the Rakefile invokes judges test --no-log --disable live --lib lib judges. A renamed or removed flag breaks the container at runtime, not at bundle install, and there is no test in this repo that would catch it (rake judges uses the same flags, so it fails together with production rather than ahead of it). That is precisely the failure mode the description names -- and ~>0 does not prevent it, because such a rename ships as 0.60 -> 0.61.

This diff carries the evidence. judges moves 0.58.1 -> 0.60.4 in the lockfile, and that minor range added two new runtime dependencies: ellipsized ~> 0.3 and octokit ~> 10.0 (lock lines 120 and 127). Current released judges is 0.61.1 (2026-07-24), which ~>0 also admits silently.

If you want a bound that actually works for judges, ~> 0.60.4 is the one that matches the argument in the description, and it should come with a line in README.md telling downstream swarm authors to raise it deliberately after reading the judges changelog. Leaving it at ~>0 means the flag-rename scenario is still wide open while the repo now carries a constraint that reads as protection.

(require: false is correct and unaffected -- judges is only ever shelled out to, never required.)

gem 'minitest', '~>6.0', require: false

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Every other constrained gem in this file uses a two-segment pessimistic bound: ~>6.0 on this line, then ~>1.7, ~>13.2, ~>1.75, ~>0.22, ~>3.0. ~>0 is the only single-segment constraint in the file, and single-segment ~> is rare enough in practice that most readers will parse it as "pinned to 0.x somehow" rather than as the precise >= 0, < 1 it is. If a bound stays at all, writing it as ~>0.48 / ~>0.60 costs nothing and is self-documenting -- a reader can see which release the constraint was reasoned about against, and git blame on the line gains meaning.

There is also an internal inconsistency this PR leaves in place. Three gems on lines 14-16 are still >0: rubocop-minitest, rubocop-performance, rubocop-rake. rubocop-minitest is pre-1.0 -- open renovate PR #294 raises it to 0.40.0 -- so the "pre-1.0 gems may break on minor bumps" argument applies to it verbatim, and it is a gem that gates the build via rake rubocop. Either the argument justifies bounding all pre-1.0 dependencies or it justifies bounding none; picking two and leaving a third makes the policy unreadable to whoever edits this file next.

This is worth resolving explicitly rather than by omission, because in a template repo this file is the policy statement that every generated swarm inherits.

gem 'minitest-reporters', '~>1.7', require: false
gem 'rake', '~>13.2', require: false

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This changes how renovate treats these two lines, and the repo already contains a controlled experiment for both behaviours. renovate.json is a bare config:recommended with no rangeStrategy set, so the bundler manager uses its default: a gem whose constraint already admits the new version gets a Gemfile.lock-only PR, and a gem whose constraint excludes it gets a PR that rewrites the Gemfile line too. Compare open PR #294 (rubocop-minitest to 0.40.0, constraint >0) which touches Gemfile.lock only, against #291 (simplecov to v1, constraint ~>0.22) and #292 (simplecov-cobertura to v4, constraint ~>3.0), both of which rewrite Gemfile.

So the effect of ~>0 is deferred rather than absent. While fbe and judges stay in 0.x, renovate keeps sending lockfile-only PRs exactly as it does today (#288 for fbe 0.49.1, #289 for judges 0.61.1, both Gemfile.lock-only) -- no new churn. On the day either gem cuts 1.0, renovate opens a PR that changes ~>0 to ~>1.0 and relocks, and a maintainer merges it the same way #291 and #292 will be merged: without the changelog review the bound existed to force.

That is the honest cost/benefit -- the bound buys one automated pull request's worth of friction, priced against a constraint that must be maintained here and in every generated swarm. If a real review gate is what is wanted, put it in renovate.json where it is legible: "rangeStrategy": "update-lockfile" scoped to these two packages, or "dependencyDashboardApproval": true for them, both of which state the intent instead of encoding it in a version range that a bot will rewrite.

Expand Down
76 changes: 42 additions & 34 deletions Gemfile.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 8 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ based on business rules. Judges run in cycles until no new facts are produced.

## Project Structure

```
```text

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This whole README hunk -- the text language on this fence, the four 1. renumberings, the table delimiter row, and the MD013 disable pair -- is byte-for-byte identical to open PR #290 by the same author ("#284 fix markdownlint violations in README.md"), which touches README.md and nothing else. Two open PRs carrying the same diff means whichever merges second conflicts, and the same eight lines get reviewed twice by whoever ends up reviewing both.

Drop README.md from this PR and let #290 land it. That also removes the only reason markdown-lint is green here: the check is currently failing on master (10 errors), and bundling the fix into a dependency PR means the red build gets repaired as a side effect of an unrelated change, under the wrong issue number, with the repair invisible in the PR title. It should be fixed by the PR that exists for exactly that purpose.

The fence change itself is correct -- markdownlint reports MD040/fenced-code-language on this line against master, and text is the right choice for a directory listing rather than inventing a language.

judges/<name>/<name>.rb — judge implementation, auto-discovered
judges/<name>/<name>.yml — YAML test for the judge (data-driven)
lib/ — shared Ruby libraries
Expand All @@ -41,7 +41,7 @@ bundle exec rake
mkdir judges/hello-world
```

2. Write the judge logic in `judges/hello-world/hello-world.rb`:
1. Write the judge logic in `judges/hello-world/hello-world.rb`:

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the markdownlint-correct fix and simultaneously a rendering regression. MD029/ol-prefix fires on master at lines 44, 59, 77 and 83 because each fenced code block sits at column 0, which terminates the list -- so 2., 3., 4. and 5. are each the first item of a brand-new ordered list, and markdownlint requires a list to start at 1. Renumbering them all to 1. silences the rule.

But GitHub honours the start number of an ordered list. 2. Write the judge logic currently renders as step 2; 1. Write the judge logic renders as step 1. After this change the "Adding a New Judge" section shows five consecutive steps all labelled "1.", which is a worse document than the one with the lint warnings -- and this is the section that teaches every downstream swarm author how to write their first judge.

The fix that satisfies both is to make it a single list again by indenting each fenced block three spaces so it sits inside its preceding list item. Then the numbering can stay 1. through 5.: MD029's default one_or_ordered style accepts a properly incrementing list, and the rendered page reads as five steps instead of five step-ones. The same change is needed at lines 59, 77 and 83.


```ruby
# frozen_string_literal: true
Expand All @@ -56,7 +56,7 @@ Fbe.fb.query('(and (exists hi) (absent hello))').each do |f|
end
```

3. Write a YAML test in `judges/hello-world/hello-world.yml`:
1. Write a YAML test in `judges/hello-world/hello-world.yml`:

```yaml
# SPDX-FileCopyrightText: Copyright (c) 2024-2026 Zerocracy
Expand All @@ -74,13 +74,13 @@ expected:
- /fb/f[_id=1]/hello
```

4. Run the judge YAML tests:
1. Run the judge YAML tests:

```bash
bundle exec judges test --no-log --disable live --lib lib judges
```

5. Run the full build:
1. Run the full build:

```bash
bundle exec rake
Expand All @@ -90,13 +90,15 @@ If everything is clean, your judge is ready.

## Commands

<!-- markdownlint-disable MD013 -->

@Favixx Favixx Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The blanket disable is considerably wider than the problem. Running markdownlint against master reports exactly one MD013/line-length violation in this file -- the bundle exec judges test --no-log --disable live --lib lib judges row, at 89 characters against the default limit of 80. This pair suppresses the rule across all five table rows, including the four that are comfortably under, so a future row that runs to 200 characters lands silently.

In a template repository this is also the wrong place for the setting. There is no .markdownlint.json or .markdownlint-cli2.jsonc in the repo, so DavidAnson/markdownlint-cli2-action runs on pure defaults. Adding a config with MD013: { "tables": false } (or a raised line_length) fixes the class of problem once, applies to every markdown file including the ones downstream swarm authors add, and is inherited cleanly. The inline disable/enable pair instead gets copy-pasted into every generated swarm's README, where nobody will remember what it was for.

The delimiter reformat on line 95 is a genuine fix and should stay wherever this hunk lands -- MD060/table-column-style fires four times on master for the missing spaces around those pipes.

| Command | Purpose |
|---------|---------|
| ------- | ------- |
| `bundle exec rake` | Full build (test + judges + rubocop) |
| `bundle exec rake test` | Ruby unit tests only |
| `bundle exec judges test --no-log --disable live --lib lib judges` | Judge YAML tests |
| `bundle exec rubocop` | Code style check |
| `docker build -t swarm .` | Build Docker image (requires Dockerfile) |
<!-- markdownlint-enable MD013 -->

## How to Contribute

Expand Down
Loading