mirror of
https://github.com/garrytan/gstack.git
synced 2026-06-17 07:10:12 +02:00
fix(review): pre-emit verification gate kills Django-shape FP class (#1539)
External user filed 4/8 false positives on a /review run against a Django +
DRF + PostgreSQL repo (Sprint 2.5). Every FP class was the same shape:
"resolvable in <5 minutes by viewing the actual code or running a simple
grep" — fields that don't exist on the model, dict.get()-might-be-None on a
form that returns {}-initialized cleaned_data, standard ORM save behavior
called out as data loss.
Extends the Confidence Calibration resolver (consumed by review, cso,
plan-eng-review, ship) with a Pre-emit verification gate:
Every finding MUST quote the specific code line that motivates it
(file:line + verbatim text). If the reviewer cannot produce the quote,
the finding is unverified — its confidence is forced to 4-5 so the
existing "Suppress from main report" rule fires automatically. The
finding still goes to the appendix for calibration audit, but the user
does not see it in the critical-pass output.
Reuses the existing suppression mechanism — no new code path. The FP
classes the gate kills are enumerated in the resolver text so reviewers
see the named patterns.
Framework-meta nudge included for Django Meta, Rails associations,
SQLAlchemy relationships, TypeORM decorators, Sequelize init, Prisma
generated client — the reviewer must quote the meta-construct that
generates the symbol, not just grep for the literal name. Deeper
framework-aware ORM verification (model introspection, migration-history-
aware checks) is deliberately deferred to a future wave per T-Codex-2.
Atomic .tmpl-equivalent (resolver) edit + gen:skill-docs regen commit
per T-Codex-3.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1272,6 +1272,43 @@ Example:
|
||||
\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\`
|
||||
\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\`
|
||||
|
||||
### Pre-emit verification gate (#1539 — kills the "field doesn't exist" FP class)
|
||||
|
||||
Before any finding is promoted to the report, the gate requires:
|
||||
|
||||
1. **Quote the specific code line that motivates the finding** — file:line plus
|
||||
the verbatim text of the line(s) that triggered it. If the finding is "field
|
||||
X doesn't exist on model Y", quote the lines of class Y where the field
|
||||
would live. If "dict.get() might return None", quote the dict initialization.
|
||||
If "race condition between A and B", quote both A and B.
|
||||
|
||||
2. **If you cannot quote the motivating line(s), the finding is unverified.**
|
||||
Force its confidence to 4-5 (suppressed from the main report). It still goes
|
||||
into the appendix so reviewers can audit calibration, but the user does NOT
|
||||
see it in the critical-pass output. Do not work around this by inventing
|
||||
speculative confidence 7+ — that defeats the gate.
|
||||
|
||||
**Framework-meta nudge:** When the symbol is generated by a framework
|
||||
metaclass, descriptor, ORM Meta inner-class, or migration history (Django
|
||||
`Meta`, Rails `has_many`/`scope`, SQLAlchemy `relationship`/`Column`,
|
||||
TypeORM decorators, Sequelize `init`/`belongsTo`, Prisma generated client),
|
||||
quote the meta-construct (the `Meta` block, the migration, the decorator,
|
||||
the schema file) instead of expecting the literal name in the class body.
|
||||
The verification is "I read the source that creates this symbol", not "I
|
||||
grep'd for the name and didn't find it." Deeper framework-aware verification
|
||||
(model introspection, migration-history-aware checks, ORM dialect detection)
|
||||
is deliberately out of scope for the lighter gate — see the deferred
|
||||
`~/.gstack-dev/plans/1539-framework-aware-review.md` design doc.
|
||||
|
||||
The FP classes the gate kills (measured against Django Sprint 2.5 #1539):
|
||||
|
||||
| FP class | Why the gate catches it |
|
||||
|---|---|
|
||||
| "field doesn't exist on model" | Requires quoting the model class body or Meta; the field's absence becomes obvious |
|
||||
| "dict.get() might be None" | Requires quoting the dict initialization (e.g. Django form's `cleaned_data` is `{}`-initialized) |
|
||||
| "save() might lose fields" | Requires quoting the ORM signature or model definition |
|
||||
| "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident |
|
||||
|
||||
**Calibration learning:** If you report a finding with confidence < 7 and the user
|
||||
confirms it IS a real issue, that is a calibration event. Your initial confidence was
|
||||
too low. Log the corrected pattern as a learning so future reviews catch it with
|
||||
|
||||
@@ -992,6 +992,43 @@ Example:
|
||||
\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\`
|
||||
\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\`
|
||||
|
||||
### Pre-emit verification gate (#1539 — kills the "field doesn't exist" FP class)
|
||||
|
||||
Before any finding is promoted to the report, the gate requires:
|
||||
|
||||
1. **Quote the specific code line that motivates the finding** — file:line plus
|
||||
the verbatim text of the line(s) that triggered it. If the finding is "field
|
||||
X doesn't exist on model Y", quote the lines of class Y where the field
|
||||
would live. If "dict.get() might return None", quote the dict initialization.
|
||||
If "race condition between A and B", quote both A and B.
|
||||
|
||||
2. **If you cannot quote the motivating line(s), the finding is unverified.**
|
||||
Force its confidence to 4-5 (suppressed from the main report). It still goes
|
||||
into the appendix so reviewers can audit calibration, but the user does NOT
|
||||
see it in the critical-pass output. Do not work around this by inventing
|
||||
speculative confidence 7+ — that defeats the gate.
|
||||
|
||||
**Framework-meta nudge:** When the symbol is generated by a framework
|
||||
metaclass, descriptor, ORM Meta inner-class, or migration history (Django
|
||||
`Meta`, Rails `has_many`/`scope`, SQLAlchemy `relationship`/`Column`,
|
||||
TypeORM decorators, Sequelize `init`/`belongsTo`, Prisma generated client),
|
||||
quote the meta-construct (the `Meta` block, the migration, the decorator,
|
||||
the schema file) instead of expecting the literal name in the class body.
|
||||
The verification is "I read the source that creates this symbol", not "I
|
||||
grep'd for the name and didn't find it." Deeper framework-aware verification
|
||||
(model introspection, migration-history-aware checks, ORM dialect detection)
|
||||
is deliberately out of scope for the lighter gate — see the deferred
|
||||
`~/.gstack-dev/plans/1539-framework-aware-review.md` design doc.
|
||||
|
||||
The FP classes the gate kills (measured against Django Sprint 2.5 #1539):
|
||||
|
||||
| FP class | Why the gate catches it |
|
||||
|---|---|
|
||||
| "field doesn't exist on model" | Requires quoting the model class body or Meta; the field's absence becomes obvious |
|
||||
| "dict.get() might be None" | Requires quoting the dict initialization (e.g. Django form's `cleaned_data` is `{}`-initialized) |
|
||||
| "save() might lose fields" | Requires quoting the ORM signature or model definition |
|
||||
| "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident |
|
||||
|
||||
**Calibration learning:** If you report a finding with confidence < 7 and the user
|
||||
confirms it IS a real issue, that is a calibration event. Your initial confidence was
|
||||
too low. Log the corrected pattern as a learning so future reviews catch it with
|
||||
|
||||
@@ -1202,6 +1202,43 @@ Example:
|
||||
\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\`
|
||||
\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\`
|
||||
|
||||
### Pre-emit verification gate (#1539 — kills the "field doesn't exist" FP class)
|
||||
|
||||
Before any finding is promoted to the report, the gate requires:
|
||||
|
||||
1. **Quote the specific code line that motivates the finding** — file:line plus
|
||||
the verbatim text of the line(s) that triggered it. If the finding is "field
|
||||
X doesn't exist on model Y", quote the lines of class Y where the field
|
||||
would live. If "dict.get() might return None", quote the dict initialization.
|
||||
If "race condition between A and B", quote both A and B.
|
||||
|
||||
2. **If you cannot quote the motivating line(s), the finding is unverified.**
|
||||
Force its confidence to 4-5 (suppressed from the main report). It still goes
|
||||
into the appendix so reviewers can audit calibration, but the user does NOT
|
||||
see it in the critical-pass output. Do not work around this by inventing
|
||||
speculative confidence 7+ — that defeats the gate.
|
||||
|
||||
**Framework-meta nudge:** When the symbol is generated by a framework
|
||||
metaclass, descriptor, ORM Meta inner-class, or migration history (Django
|
||||
`Meta`, Rails `has_many`/`scope`, SQLAlchemy `relationship`/`Column`,
|
||||
TypeORM decorators, Sequelize `init`/`belongsTo`, Prisma generated client),
|
||||
quote the meta-construct (the `Meta` block, the migration, the decorator,
|
||||
the schema file) instead of expecting the literal name in the class body.
|
||||
The verification is "I read the source that creates this symbol", not "I
|
||||
grep'd for the name and didn't find it." Deeper framework-aware verification
|
||||
(model introspection, migration-history-aware checks, ORM dialect detection)
|
||||
is deliberately out of scope for the lighter gate — see the deferred
|
||||
`~/.gstack-dev/plans/1539-framework-aware-review.md` design doc.
|
||||
|
||||
The FP classes the gate kills (measured against Django Sprint 2.5 #1539):
|
||||
|
||||
| FP class | Why the gate catches it |
|
||||
|---|---|
|
||||
| "field doesn't exist on model" | Requires quoting the model class body or Meta; the field's absence becomes obvious |
|
||||
| "dict.get() might be None" | Requires quoting the dict initialization (e.g. Django form's `cleaned_data` is `{}`-initialized) |
|
||||
| "save() might lose fields" | Requires quoting the ORM signature or model definition |
|
||||
| "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident |
|
||||
|
||||
**Calibration learning:** If you report a finding with confidence < 7 and the user
|
||||
confirms it IS a real issue, that is a calibration event. Your initial confidence was
|
||||
too low. Log the corrected pattern as a learning so future reviews catch it with
|
||||
|
||||
@@ -6,6 +6,13 @@
|
||||
* 7+: show normally
|
||||
* 5-6: show with caveat
|
||||
* <5: suppress from main report
|
||||
*
|
||||
* Pre-emit verification gate (#1539): findings without a quoted code snippet
|
||||
* are forced to confidence 4-5 so the existing suppression rule fires
|
||||
* automatically. Kills the "field doesn't exist on the model" FP class on
|
||||
* mature frameworks like Django/Rails — the model code resolves it in <5min,
|
||||
* and the gate forces the reviewer to do that lookup before promoting the
|
||||
* finding to the report.
|
||||
*/
|
||||
import type { TemplateContext } from './types';
|
||||
|
||||
@@ -30,6 +37,43 @@ Example:
|
||||
\\\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\\\`
|
||||
\\\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\\\`
|
||||
|
||||
### Pre-emit verification gate (#1539 — kills the "field doesn't exist" FP class)
|
||||
|
||||
Before any finding is promoted to the report, the gate requires:
|
||||
|
||||
1. **Quote the specific code line that motivates the finding** — file:line plus
|
||||
the verbatim text of the line(s) that triggered it. If the finding is "field
|
||||
X doesn't exist on model Y", quote the lines of class Y where the field
|
||||
would live. If "dict.get() might return None", quote the dict initialization.
|
||||
If "race condition between A and B", quote both A and B.
|
||||
|
||||
2. **If you cannot quote the motivating line(s), the finding is unverified.**
|
||||
Force its confidence to 4-5 (suppressed from the main report). It still goes
|
||||
into the appendix so reviewers can audit calibration, but the user does NOT
|
||||
see it in the critical-pass output. Do not work around this by inventing
|
||||
speculative confidence 7+ — that defeats the gate.
|
||||
|
||||
**Framework-meta nudge:** When the symbol is generated by a framework
|
||||
metaclass, descriptor, ORM Meta inner-class, or migration history (Django
|
||||
\`Meta\`, Rails \`has_many\`/\`scope\`, SQLAlchemy \`relationship\`/\`Column\`,
|
||||
TypeORM decorators, Sequelize \`init\`/\`belongsTo\`, Prisma generated client),
|
||||
quote the meta-construct (the \`Meta\` block, the migration, the decorator,
|
||||
the schema file) instead of expecting the literal name in the class body.
|
||||
The verification is "I read the source that creates this symbol", not "I
|
||||
grep'd for the name and didn't find it." Deeper framework-aware verification
|
||||
(model introspection, migration-history-aware checks, ORM dialect detection)
|
||||
is deliberately out of scope for the lighter gate — see the deferred
|
||||
\`~/.gstack-dev/plans/1539-framework-aware-review.md\` design doc.
|
||||
|
||||
The FP classes the gate kills (measured against Django Sprint 2.5 #1539):
|
||||
|
||||
| FP class | Why the gate catches it |
|
||||
|---|---|
|
||||
| "field doesn't exist on model" | Requires quoting the model class body or Meta; the field's absence becomes obvious |
|
||||
| "dict.get() might be None" | Requires quoting the dict initialization (e.g. Django form's \`cleaned_data\` is \`{}\`-initialized) |
|
||||
| "save() might lose fields" | Requires quoting the ORM signature or model definition |
|
||||
| "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident |
|
||||
|
||||
**Calibration learning:** If you report a finding with confidence < 7 and the user
|
||||
confirms it IS a real issue, that is a calibration event. Your initial confidence was
|
||||
too low. Log the corrected pattern as a learning so future reviews catch it with
|
||||
|
||||
@@ -1921,6 +1921,43 @@ Example:
|
||||
\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\`
|
||||
\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\`
|
||||
|
||||
### Pre-emit verification gate (#1539 — kills the "field doesn't exist" FP class)
|
||||
|
||||
Before any finding is promoted to the report, the gate requires:
|
||||
|
||||
1. **Quote the specific code line that motivates the finding** — file:line plus
|
||||
the verbatim text of the line(s) that triggered it. If the finding is "field
|
||||
X doesn't exist on model Y", quote the lines of class Y where the field
|
||||
would live. If "dict.get() might return None", quote the dict initialization.
|
||||
If "race condition between A and B", quote both A and B.
|
||||
|
||||
2. **If you cannot quote the motivating line(s), the finding is unverified.**
|
||||
Force its confidence to 4-5 (suppressed from the main report). It still goes
|
||||
into the appendix so reviewers can audit calibration, but the user does NOT
|
||||
see it in the critical-pass output. Do not work around this by inventing
|
||||
speculative confidence 7+ — that defeats the gate.
|
||||
|
||||
**Framework-meta nudge:** When the symbol is generated by a framework
|
||||
metaclass, descriptor, ORM Meta inner-class, or migration history (Django
|
||||
`Meta`, Rails `has_many`/`scope`, SQLAlchemy `relationship`/`Column`,
|
||||
TypeORM decorators, Sequelize `init`/`belongsTo`, Prisma generated client),
|
||||
quote the meta-construct (the `Meta` block, the migration, the decorator,
|
||||
the schema file) instead of expecting the literal name in the class body.
|
||||
The verification is "I read the source that creates this symbol", not "I
|
||||
grep'd for the name and didn't find it." Deeper framework-aware verification
|
||||
(model introspection, migration-history-aware checks, ORM dialect detection)
|
||||
is deliberately out of scope for the lighter gate — see the deferred
|
||||
`~/.gstack-dev/plans/1539-framework-aware-review.md` design doc.
|
||||
|
||||
The FP classes the gate kills (measured against Django Sprint 2.5 #1539):
|
||||
|
||||
| FP class | Why the gate catches it |
|
||||
|---|---|
|
||||
| "field doesn't exist on model" | Requires quoting the model class body or Meta; the field's absence becomes obvious |
|
||||
| "dict.get() might be None" | Requires quoting the dict initialization (e.g. Django form's `cleaned_data` is `{}`-initialized) |
|
||||
| "save() might lose fields" | Requires quoting the ORM signature or model definition |
|
||||
| "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident |
|
||||
|
||||
**Calibration learning:** If you report a finding with confidence < 7 and the user
|
||||
confirms it IS a real issue, that is a calibration event. Your initial confidence was
|
||||
too low. Log the corrected pattern as a learning so future reviews catch it with
|
||||
|
||||
Reference in New Issue
Block a user