Skip to content

fix(review): keep from_attributes live when nested models validate ORM objects - #1383

Open
aniruddhaadak80 wants to merge 1 commit into
DietrichGebert:mainfrom
aniruddhaadak80:fix/pydantic-from-attributes-liveness
Open

aniruddhaadak80 wants to merge 1 commit into
DietrichGebert:mainfrom
aniruddhaadak80:fix/pydantic-from-attributes-liveness

Conversation

@aniruddhaadak80

Copy link
Copy Markdown
Contributor

Closes #1358

Root cause

/ponytail-review and /ponytail-audit could call code, a flag or a config key dead from a single call site. In the issue's repro, ParentResponse.model_validate(entity.model_dump()) was checked, PeriodResponse.model_config = ConfigDict(from_attributes=True) looked unused, and the second call site (ParentResponse.model_validate(entity)) was missed. Removing the flag then raised Input should be a valid dictionary or instance of PeriodResponse at runtime.

The base ponytail skill already says "grep every caller" before a bug fix, but the review and audit skills did not apply that rule to deletions.

Fix

The review and audit skills now require every use of the owner before a deletion finding: direct calls, tests, config, string and dynamic references, serialization, reflection and framework hooks. One call site where a setting does nothing does not prove it is dead. The Pydantic case is named explicitly: from_attributes=True on a nested model is live when another call site validates an ORM object, even if the first validates a dict.

When the every-use check is not possible, the finding is marked unverified and no deletion is recommended; it goes under Not checked: instead. The rule ships in all copies hosts actually load:

  • skills/ponytail-review/SKILL.md, skills/ponytail-audit/SKILL.md
  • commands/ponytail-review.toml, commands/ponytail-audit.toml
  • .opencode/command/ponytail-review.md, .opencode/command/ponytail-audit.md
  • generated .openclaw/skills/ponytail-{review,audit}/SKILL.md

Tests

Added tests/delete-liveness.test.js with the issue's Pydantic repro in the header. It fails if any shipped copy of review/audit:

  • stops requiring every use of the owner, not one call site
  • drops the from_attributes / ORM-object failure mode
  • loses the unverified escape hatch or recommends deleting an unverified finding
node --test tests/delete-liveness.test.js
# 24 pass, 0 fail

npm test
# root: 166 pass, 2 skipped, 0 fail
# pi-extension: 30 pass, 0 fail

node scripts/check-rule-copies.js
# Rule copies match AGENTS.md; 10 rule invariants present

No baseline failures. The 2 root-test skips are pre-existing platform skips (grep/sh unavailable paths).

…rt#1358)

ponytail-review and ponytail-audit marked a live Pydantic from_attributes=True dead after checking one model_validate(dict) call site. A second call site, model_validate(orm_object), needs the flag for nested models, so removing it broke the endpoint at runtime.

Both skills now require every use of the owner before a deletion finding: direct calls, tests, config, string and dynamic references, serialization, reflection and framework hooks. When those cannot all be checked, the finding is unverified and no deletion is recommended. The rule ships in the canonical skills, the Claude/Gemini commands, the OpenCode commands and the generated OpenClaw skills, with a regression test pinning the issue's repro.
Copilot AI balanced review requested due to automatic review settings November 10, 2026 04:56

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

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.

ponytail-review marks a live Pydantic from_attributes flag as dead after checking only one call site

2 participants