Benchmark PR 8

[celmis-codereviewer]

Benchmark reproduction of ai-code-review-evaluation#8

[celmis-codereviewer]

❌ CHANGES REQUESTED — blocking findings

Full findings and scope are in the review summary comment on this pull request — one persistent comment, updated in place on every run.

[celmis-codereviewer]

Why: Line 21 uses {{each ...}} without the # prefix required for Handlebars/Ember block helpers, causing a template syntax error when Ember attempts to compile group.hbs.

🔴 Missing # prefix on Ember block helper {{each}}

Handlebars block helpers in Ember require a # prefix (e.g. {{#each member in members itemView="group-member"}}). Using {{each member in members ...}} without # creates invalid syntax that fails to parse or compile at runtime when rendering the view.

Suggested change

	{{each member in members itemView="group-member"}}
	{{#each member in members itemView="group-member"}}{{/each}}

_{agent: defect · rule: defect.syntax-error · confidence: 0.95}

[celmis-codereviewer]

Why: Line 90 passes user_id (an integer) to group.users.delete, which expects User record instances for ActiveRecord associations and raises ActiveRecord::AssociationTypeMismatch when executed.

🔴 Type mismatch when deleting member from ActiveRecord association

group.users.delete(user_id) passes user_id (an Integer) directly to group.users.delete. ActiveRecord has_many :through association delete methods require User model instances as arguments. Passing an integer ID causes an ActiveRecord::AssociationTypeMismatch exception when deleting group members.

Suggested change

	group.users.delete(user_id)
	if user = User.find_by(id: user_id)
	group.remove(user)
	end

_{agent: defect · rule: defect.type-mismatch · confidence: 0.95}

[celmis-codereviewer]

Why: user_count divided by limit plus 1 in totalPages on line 13 yields 2 when user_count is 50 and limit is 50, falsely reporting a second page when only 1 page exists.

🟠 Off-by-one error in totalPages calculation

In totalPages, Math.floor(this.get("user_count") / this.get("limit")) + 1 evaluates to 1 + 1 = 2 when user_count is 50 and limit is 50. Because 50 users fit on a single page of size 50, totalPages should be 1. This formula overcounts total pages by 1 whenever user_count is a non-zero multiple of limit, which keeps the pagination control enabled when no further pages exist.

Suggested change

	return Math.floor(this.get("user_count") / this.get("limit")) + 1;
	totalPages: function() {
	if (this.get("user_count") == 0) { return 0; }
	return Math.ceil(this.get("user_count") / this.get("limit"));
	}.property("limit", "user_count"),

_{agent: defect · rule: defect.off-by-one · confidence: 0.95}

[celmis-codereviewer]

Why: params.require(:group_id) on line 66 will raise ActionController::ParameterMissing at runtime because the resource route defined on line 50 of config/routes.rb captures the group ID as params[:id].

🟠 Parameter mismatch between resource route parameter :id and controller expectation :group_id

The member routes defined inside resources :groups in config/routes.rb capture the resource ID in params[:id]:

delete "members" => "groups#remove_member"
put "members" => "groups#add_members"

However, Admin::GroupsController#add_members requires :group_id instead of :id:

group = Group.find(params.require(:group_id).to_i)

When requests from Discourse.Group.prototype.addMembers hit /admin/groups/:id/members.json, params[:group_id] will be missing, causing ActionController::ParameterMissing to be raised.

Also at line 84.

Suggested change

	group = Group.find(params.require(:group_id).to_i)
	group = Group.find(params.require(:id).to_i)

_{agent: contract · rule: contract.parameter-mismatch · confidence: 0.95}

[celmis-codereviewer]

🤖 Code Review for PR #18

⚙ ADJUSTED — graph context partial (13 of 16 changed files): 3 of 16 changed files have no symbols in the index; 3 of them are not in that checkout at all (app/assets/javascripts/admin/routes/admin_group_route.js, app/assets/javascripts/discourse/models/group.js, spec/controllers/admin/groups_controller_spec.rb) — this PR's base is older than the indexed revision, so those files were renamed or deleted before it and no re-index can bring them back; there is nothing to fix.

❌ CHANGES REQUESTED — blocking findings

Findings

• 🔴 Critical: 2
• 🟠 Error: 2

Scope

• Files changed: 16
• Lines: +389 / -283

Performance

• Analysis time: 287.6s · agents: structural, cve, security, contract, defect · tokens: 47,501/39,755

---

_{Powered by Code Analyzer · context: tree-sitter graph + structural, cve, security, contract, defect}