Code Review Bench PR #25477 - Removed unused tinybird filters

[ketkarameya]

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#12

Removes unused Tinybird filter configurations to clean up codebase.

[gitar-bot-staging]

🚨 Bug: Storing full tokenData object breaks caching and auth

The change from this._serverToken = tokenData.token to this._serverToken = tokenData stores the full {token, exp} object instead of the JWT string. This causes two cascading failures:

1. Token caching is broken: On line 96, this._isJWTExpired(this._serverToken) passes the object to jwt.decode(), which expects a string. jwt.decode({token, exp}) returns null, so _isJWTExpired always returns true, causing a new token to be generated on every getToken() call.

2. API authentication is broken: On line 102, token: this._serverToken nests the object, returning {token: {token: "jwt...", exp: 12345}, exp: 12345}. Consumers like services/stats/utils/tinybird.js do const token = tokenData?.token, getting the {token, exp} object instead of a string. This makes Authorization: Bearer ${token} resolve to Bearer [object Object], causing all Tinybird API requests to fail with authentication errors.

Similarly, api/endpoints/tinybird.js returns tokenData.token to the API response, which would be the nested object instead of a string.

Fix: Revert this line back to this._serverToken = tokenData.token; — the original code was correct.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	this._serverToken = tokenData;
	this._serverToken = tokenData.token;

• Apply suggested fix

[gitar-bot-staging]

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}

[gitar-bot-staging]

Note

Gitar has paused automatic reviews. Comment "Gitar review" to trigger a review manually.

Code Review 🚫 Blocked 0 resolved / 2 findings

Critical bug: storing the full tokenData object instead of tokenData.token breaks JWT caching and causes all Tinybird API requests to fail with Bearer [object Object] authentication. The filter cleanup itself is clean and thorough.

🚨 Bug: Storing full tokenData object breaks caching and auth

📄 ghost/core/core/server/services/tinybird/TinybirdService.js:98

The change from this._serverToken = tokenData.token to this._serverToken = tokenData stores the full {token, exp} object instead of the JWT string. This causes two cascading failures:

1. Token caching is broken: On line 96, this._isJWTExpired(this._serverToken) passes the object to jwt.decode(), which expects a string. jwt.decode({token, exp}) returns null, so _isJWTExpired always returns true, causing a new token to be generated on every getToken() call.

2. API authentication is broken: On line 102, token: this._serverToken nests the object, returning {token: {token: "jwt...", exp: 12345}, exp: 12345}. Consumers like services/stats/utils/tinybird.js do const token = tokenData?.token, getting the {token, exp} object instead of a string. This makes Authorization: Bearer ${token} resolve to Bearer [object Object], causing all Tinybird API requests to fail with authentication errors.

Similarly, api/endpoints/tinybird.js returns tokenData.token to the API response, which would be the nested object instead of a string.

Fix: Revert this line back to this._serverToken = tokenData.token; — the original code was correct.

Suggested fix

                this._serverToken = tokenData.token;

💡 Quality: ARCHITECTURE.md still references removed pipes

📄 ghost/core/core/server/services/tinybird/TinybirdService.js:48

ARCHITECTURE.md still references api_top_browsers, api_top_devices, and api_top_os pipes that were removed from TINYBIRD_PIPES in this PR. The documentation should be updated to reflect the removal of these pipes to keep docs in sync with the code.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[kayagokalp]

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 2/5 = 40.0%)

ID	Severity	Title	Found?
GT-12-001	Critical	Storing entire tokenData object instead of token string	Yes
GT-12-002	Important	Removed noTimestamp option from JWT signing	No
GT-12-003	Critical	Changed jwt.verify to jwt.decode, removing signature verific	No
GT-12-004	Important	Documentation recommends npm instead of Yarn	No
GT-12-005	Important	ARCHITECTURE.md not updated to reflect removed pipes	Yes

Extra Gitar Findings

File	Line	Valid Bug?	Notes
(none)	-	-	-

Summary

• Recall: 40.0% (2/5)
• Precision: 100.0% (2/2)
• Precision counts ground truth matches + valid extra findings as true positives