Rename AI verdict columns to neutral semantics #102

Manually merged
marvin8 merged 3 commits from feat/issue-100-rename-ai-verdict-columns into feat/issue-100-generic-ai-classification 2026-08-22 05:22:29 +00:00
Collaborator

Sub-PR 2 of 3 for the domain-agnostic AI classification effort (#100).

What

  • One alembic migration (c4e8a1f3b7d2) renaming posts.text_is_promotionaltext_flagged and posts.vision_is_real_catvision_accepted — rename only, data preserved, downgrade restores the old names.
  • models.py mapped attributes and schemas.py REST response fields follow the new names (breaking API change; Zhongli reads none of these fields — verified).
  • Pipeline assignments (ai_pipeline.py) swept to the new attribute names.

Tests (TDD)

  • Migration round-trip: upgrade from pre-rename head 3d17cba4ae82 with seeded rows via old column names → upgrade to head → values preserved under new names, old names gone; downgrade restores.
  • Model metadata: neutral names present, cat-era names absent.
  • REST response: text_flagged / vision_accepted exposed, old keys dropped.
  • Pipeline tests renamed to the new attributes.

Refs #100

Sub-PR 2 of 3 for the domain-agnostic AI classification effort (#100). ## What - One alembic migration (`c4e8a1f3b7d2`) renaming `posts.text_is_promotional` → `text_flagged` and `posts.vision_is_real_cat` → `vision_accepted` — rename only, data preserved, downgrade restores the old names. - `models.py` mapped attributes and `schemas.py` REST response fields follow the new names (breaking API change; Zhongli reads none of these fields — verified). - Pipeline assignments (`ai_pipeline.py`) swept to the new attribute names. ## Tests (TDD) - Migration round-trip: upgrade from pre-rename head `3d17cba4ae82` with seeded rows via old column names → upgrade to head → values preserved under new names, old names gone; downgrade restores. - Model metadata: neutral names present, cat-era names absent. - REST response: `text_flagged` / `vision_accepted` exposed, old keys dropped. - Pipeline tests renamed to the new attributes. Refs #100
♻️ rename AI verdict columns to neutral semantics
All checks were successful
/ gitleaks (pull_request) Successful in 18s
/ checks (pull_request) Successful in 2m49s
/ publish (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ pr-review (pull_request) Successful in 6m17s
229607af4e
posts.text_is_promotional -> text_flagged and
posts.vision_is_real_cat -> vision_accepted across models,
schemas, pipeline, and REST responses, with a data-preserving
alembic migration (and downgrade).
forgejo-actions left a comment

WuMing

Found 1 issue(s). See inline comments below.

## WuMing Found **1** issue(s). See inline comments below.
@ -20,6 +21,10 @@ The posts API's `reviewer_notes` field is now `auto_reject_reason`. The old name
---
The two AI verdict fields in the posts API have new, topic-neutral names: `text_is_promotional` is now `text_flagged`, and `vision_is_real_cat` is now `vision_accepted`. Since the classification prompts are configurable, the API should not assume the curated topic is cats — a book-curation instance flags text for opposite reasons, and "accepted" describes what the vision stage decided without naming the subject. Existing databases keep their data through the rename via a migration; only code reading these field names needs updating.

docs [LOW]

The phrase 'flags text for opposite reasons' is vague and potentially misleading; a book-curation instance could flag text for different (not necessarily opposite) topic-specific criteria. Consider rewording to 'may flag text for entirely different reasons' or 'for different topic-specific reasons'.

**docs** [LOW] The phrase 'flags text for opposite reasons' is vague and potentially misleading; a book-curation instance could flag text for different (not necessarily opposite) topic-specific criteria. Consider rewording to 'may flag text for entirely different reasons' or 'for different topic-specific reasons'. <!-- wuming:sha256:167bae65759d81c812a10cf5b943a6a4f295c99650f992e164181459e6e61bf1 -->
📝 clarify release-note wording for topic-specific flags
All checks were successful
/ gitleaks (pull_request) Successful in 14s
/ checks (pull_request) Successful in 2m25s
/ publish (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ pr-review (pull_request) Successful in 4m4s
a0b769785b
Author
Collaborator

packages/fenliu/Release-Notes.md

The phrase 'flags text for opposite reasons' is vague and potentially misleading…

Fixed in commit a0b7697 — now reads "a book-curation instance may flag text for entirely different reasons".

[`packages/fenliu/Release-Notes.md`](https://forge.marvin8.zone/marvin8/dujiangyan/pulls/102#issuecomment-1850) > The phrase 'flags text for opposite reasons' is vague and potentially misleading… ✅ Fixed in commit `a0b7697` — now reads "a book-curation instance may flag text for entirely different reasons".
forgejo-actions left a comment

WuMing

Found 2 issue(s). See inline comments below.

## WuMing Found **2** issue(s). See inline comments below.
@ -20,6 +21,10 @@ The posts API's `reviewer_notes` field is now `auto_reject_reason`. The old name
---
The two AI verdict fields in the posts API have new, topic-neutral names: `text_is_promotional` is now `text_flagged`, and `vision_is_real_cat` is now `vision_accepted`. Since the classification prompts are configurable, the API should not assume the curated topic is cats — a book-curation instance may flag text for entirely different reasons, and "accepted" describes what the vision stage decided without naming the subject. Existing databases keep their data through the rename via a migration; only code reading these field names needs updating.

docs [LOW]

Formatting inconsistency: the renamed field names in this prose paragraph are not wrapped in backticks, unlike the same field names in the bullet on line 9 and elsewhere in the release notes. Wrap text_is_promotional, text_flagged, vision_is_real_cat, and vision_accepted in backticks for consistent code formatting.

**docs** [LOW] Formatting inconsistency: the renamed field names in this prose paragraph are not wrapped in backticks, unlike the same field names in the bullet on line 9 and elsewhere in the release notes. Wrap `text_is_promotional`, `text_flagged`, `vision_is_real_cat`, and `vision_accepted` in backticks for consistent code formatting. <!-- wuming:sha256:27f9094836a39d0e12b588b8a4cf41fb1ef93c72e53a664472b5d908aaafb54f -->
marvin8 marked this conversation as resolved
@ -395,0 +455,4 @@
@test("Downgrade restores the original AI verdict column names")
def test_downgrade_restores_names() -> None:
"""Downgrading from head puts the cat-era column names back with data intact."""

tests [LOW]

The downgrade test's docstring claims data remains intact, but the test never seeds a row before downgrade and adds no data-preservation assertions. Either seed a row and verify its values after downgrade, or narrow the docstring to only claim column names are restored.

**tests** [LOW] The downgrade test's docstring claims data remains intact, but the test never seeds a row before downgrade and adds no data-preservation assertions. Either seed a row and verify its values after downgrade, or narrow the docstring to only claim column names are restored. <!-- wuming:sha256:344d24c32942114ec6dbafc4fbcff6a80e9ec3247ceeb394fc9414b1d034cd55 -->
verify data survives the downgrade rename
All checks were successful
/ gitleaks (pull_request) Successful in 17s
/ checks (pull_request) Successful in 2m45s
/ publish (pull_request) Has been skipped
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ pr-review (pull_request) Successful in 4m8s
d61db4c4a2
Author
Collaborator

Fixed — downgrade test overclaimed data preservation

tests/test_database.py

The downgrade test's docstring claims data remains intact, but the test never seeds a row before downgrade…

Fixed in commit d61db4c — the test now seeds a row at head (new column names), downgrades, and asserts the values survive under the restored old names, so the docstring's claim is actually verified.


🔴 Not actioned — field names are already backticked

packages/fenliu/Release-Notes.md

the renamed field names in this prose paragraph are not wrapped in backticks…

The four field names in that paragraph are wrapped in backticks in both revisions of this file in the PR (229607a and current): `text_is_promotional`, `text_flagged`, `vision_is_real_cat`, `vision_accepted` — matching the bullet on line 9. No change needed.

### ✅ Fixed — downgrade test overclaimed data preservation [`tests/test_database.py`](https://forge.marvin8.zone/marvin8/dujiangyan/pulls/102#issuecomment-1855) > The downgrade test's docstring claims data remains intact, but the test never seeds a row before downgrade… ✅ Fixed in commit `d61db4c` — the test now seeds a row at head (new column names), downgrades, and asserts the values survive under the restored old names, so the docstring's claim is actually verified. --- ### 🔴 Not actioned — field names are already backticked [`packages/fenliu/Release-Notes.md`](https://forge.marvin8.zone/marvin8/dujiangyan/pulls/102#issuecomment-1854) > the renamed field names in this prose paragraph are not wrapped in backticks… The four field names in that paragraph are wrapped in backticks in both revisions of this file in the PR (`229607a` and current): `` `text_is_promotional` ``, `` `text_flagged` ``, `` `vision_is_real_cat` ``, `` `vision_accepted` `` — matching the bullet on line 9. No change needed.
marvin8 approved these changes 2026-08-22 05:07:21 +00:00
marvin8 manually merged commit 7ea648e397 into feat/issue-100-generic-ai-classification 2026-08-22 05:22:29 +00:00
marvin8 deleted branch feat/issue-100-rename-ai-verdict-columns 2026-08-22 05:24:16 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
marvin8/dujiangyan!102
No description provided.