AntFleet

Disagreement · f3aba81f-anthropic-1

Target registration field name `first_seen_below_at` is misleading for `side=below` targets

mismatch
repo 6f7fc663·PR #23·reviewed 1 week ago

Primary finding

Target registration field name `first_seen_below_at` is misleading for `side=below` targets

lowdocs-gaphigh
  • skills/price-threshold-alert/SKILL.md:71-78
  • skills/price-threshold-alert/SKILL.md:146-148
  • skills/price-threshold-alert/SKILL.md:213-215
The field is always populated regardless of side, but its name encodes only the `side=above` case (price 'below' target). For a `side=below` target (current > target), the field actually records 'first seen above'. This will confuse future maintainers and may cause bugs when someone interprets the field literally.

Recommendation

Rename to `first_observed_at` (neutral) or split into two fields. Update the LRU sort key reference in Step 8.

Counterpart finding

State write pathway vs. fetch-fail behavior and .bak handling are underspecified/inconsistent

lowmaintainabilitymedium
  • skills/price-threshold-alert/SKILL.md:96
  • skills/price-threshold-alert/SKILL.md:198-209
  • skills/price-threshold-alert/SKILL.md:211
The spec says fetch failure should do “no state mutation beyond touching last_run_at,” but the persist step shows a full-object rewrite requiring multiple variables and mentions restoring from a .bak without specifying when/how the .bak is created. The partial-update path is unclear and can lead to diverging implementations.

Recommendation

Document a minimal write path for failures (only update last_run_at, using the existing JSON as the source), and explicitly define when a .bak is created (e.g., cp before write) and restored.

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →