Skip to content

Commit f4e3695

Browse files
committed
chore: remove redundant comments and clean up code comments
Comment-only cleanup; no logic or executable code touched. - Removed ~25 decorative banner comments (`# =====`, `// -----`) that added no information beyond what the adjacent `describe(...)` / `class Test...` / function signature already conveyed. - Removed ~6 stale "Issue N" / "TODO" task references in tests and scripts that belong in git history, not source. - Collapsed one verbose H001 banner block in reporter/postgres_reports.py into a focused 3-line note about where recommendations are computed. - Removed a handful of what-comments restating obvious code (e.g., "Skip if queryid is missing" above `if not queryid: continue`). https://claude.ai/code/session_01U4F5KjW5neeCYxkKUZawfS
1 parent 222a355 commit f4e3695

14 files changed

Lines changed: 15 additions & 181 deletions

File tree

cli/bin/postgres-ai.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,6 @@ function createTtySpinner(
195195
};
196196
}
197197

198-
// ============================================================================
199-
// Checkup command helpers
200-
// ============================================================================
201-
202198
interface CheckupOptions {
203199
checkId: string;
204200
nodeName: string;
@@ -409,13 +405,6 @@ function printUploadSummary(
409405
}
410406
}
411407

412-
// ============================================================================
413-
// CLI configuration
414-
// ============================================================================
415-
416-
/**
417-
* CLI configuration options
418-
*/
419408
interface CliOptions {
420409
apiKey?: string;
421410
apiBaseUrl?: string;

cli/lib/checkup.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -955,10 +955,6 @@ function resolveBuildTs(): string | null {
955955
}
956956
}
957957

958-
// ============================================================================
959-
// Unified Report Generator Helpers
960-
// ============================================================================
961-
962958
/**
963959
* Generate a simple version report (A002, A013).
964960
* These reports only contain PostgreSQL version information.
@@ -1034,10 +1030,6 @@ async function generateIndexReport<T extends { index_size_bytes: number }>(
10341030
return report;
10351031
}
10361032

1037-
// ============================================================================
1038-
// Report Generators (using unified helpers)
1039-
// ============================================================================
1040-
10411033
/** Generate A002 report - Postgres major version */
10421034
export const generateA002 = (client: Client, nodeName = "node-01") =>
10431035
generateVersionReport(client, nodeName, "A002", "Postgres major version");

cli/lib/issues.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -670,10 +670,6 @@ export async function updateIssueComment(params: UpdateIssueCommentParams): Prom
670670
}
671671
}
672672

673-
// ============================================================================
674-
// Action Items API Functions
675-
// ============================================================================
676-
677673
export interface FetchActionItemParams {
678674
apiKey: string;
679675
apiBaseUrl: string;

cli/lib/reports.ts

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
import { formatHttpError, maskSecret, normalizeBaseUrl } from "./util";
22

3-
// ============================================================================
4-
// Types
5-
// ============================================================================
6-
73
export interface CheckupReport {
84
id: number;
95
org_id: number;
@@ -32,10 +28,6 @@ export interface CheckupReportFileData extends CheckupReportFile {
3228
data: string;
3329
}
3430

35-
// ============================================================================
36-
// Date parsing
37-
// ============================================================================
38-
3931
/**
4032
* Parse a date string in various formats into an ISO 8601 string.
4133
* Supported formats:
@@ -73,10 +65,6 @@ export function parseFlexibleDate(input: string): string {
7365
throw new Error(`Unrecognized date format: ${input}. Use YYYY-MM-DD or DD.MM.YYYY`);
7466
}
7567

76-
// ============================================================================
77-
// Params
78-
// ============================================================================
79-
8068
export interface FetchReportsParams {
8169
apiKey: string;
8270
apiBaseUrl: string;
@@ -107,10 +95,6 @@ export interface FetchReportFileDataParams {
10795
debug?: boolean;
10896
}
10997

110-
// ============================================================================
111-
// API functions
112-
// ============================================================================
113-
11498
export async function fetchReports(params: FetchReportsParams): Promise<CheckupReport[]> {
11599
const { apiKey, apiBaseUrl, projectId, status, limit = 20, beforeDate, beforeId, debug } = params;
116100
if (!apiKey) {
@@ -299,10 +283,7 @@ export async function fetchReportFileData(params: FetchReportFileDataParams): Pr
299283
}
300284
}
301285

302-
// ============================================================================
303-
// Lightweight markdown terminal renderer
304-
// ============================================================================
305-
286+
/** Lightweight markdown terminal renderer. */
306287
export function renderMarkdownForTerminal(md: string): string {
307288
if (!md) return "";
308289

cli/test/init.test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,12 +1318,8 @@ describe.skipIf(!dockerAvailable)("imageTag priority behavior", () => {
13181318
}, 60000);
13191319
});
13201320

1321-
// ---------------------------------------------------------------------------
1322-
// connectWithSslFallback — connectionTimeoutMillis and statement_timeout
1323-
// Issues 9 & 10
1324-
// ---------------------------------------------------------------------------
13251321
describe("connectWithSslFallback", () => {
1326-
// Issue 9: Verify that connectionTimeoutMillis is forwarded to the pg Client
1322+
// Verify that connectionTimeoutMillis is forwarded to the pg Client
13271323
// constructor so slow-responding servers don't hang the CLI indefinitely.
13281324
// Direct integration testing against a real TCP timeout would be flaky in CI,
13291325
// so we use a mock ClientClass and assert the config passed to its constructor.
@@ -1348,7 +1344,7 @@ describe("connectWithSslFallback", () => {
13481344
expect(receivedConfigs[0].connectionTimeoutMillis).toBe(10_000);
13491345
});
13501346

1351-
// Issue 10: Verify that SET statement_timeout is issued after every successful
1347+
// Verify that SET statement_timeout is issued after every successful
13521348
// connection to prevent runaway queries from blocking the CLI.
13531349
test("issues SET statement_timeout = '30s' after connecting", async () => {
13541350
const queriesSent: string[] = [];

cli/test/reports.cli.test.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,6 @@ async function startFakeApi() {
167167
};
168168
}
169169

170-
// ---------------------------------------------------------------------------
171-
// Help output
172-
// ---------------------------------------------------------------------------
173170
describe("CLI reports command group", () => {
174171
test("reports help exposes list, files, data subcommands", () => {
175172
const r = runCli(["reports", "--help"], isolatedEnv());
@@ -180,9 +177,6 @@ describe("CLI reports command group", () => {
180177
expect(out).toContain("data");
181178
});
182179

183-
// -----------------------------------------------------------------------
184-
// Input validation
185-
// -----------------------------------------------------------------------
186180
test("reports list fails fast when API key is missing", () => {
187181
const r = runCli(["reports", "list"], isolatedEnv());
188182
expect(r.status).toBe(1);
@@ -237,9 +231,6 @@ describe("CLI reports command group", () => {
237231
expect(`${r.stdout}\n${r.stderr}`).toContain("Either reportId or --check-id is required");
238232
});
239233

240-
// -----------------------------------------------------------------------
241-
// Successful API calls
242-
// -----------------------------------------------------------------------
243234
test("reports list succeeds against a fake API", async () => {
244235
const api = await startFakeApi();
245236
try {

cli/test/reports.test.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ import {
1010

1111
const originalFetch = globalThis.fetch;
1212

13-
// ---------------------------------------------------------------------------
14-
// fetchReports
15-
// ---------------------------------------------------------------------------
1613
describe("fetchReports", () => {
1714
afterEach(() => {
1815
globalThis.fetch = originalFetch;
@@ -274,9 +271,6 @@ describe("fetchReports", () => {
274271
});
275272
});
276273

277-
// ---------------------------------------------------------------------------
278-
// fetchAllReports
279-
// ---------------------------------------------------------------------------
280274
describe("fetchAllReports", () => {
281275
afterEach(() => {
282276
globalThis.fetch = originalFetch;
@@ -416,9 +410,6 @@ describe("fetchAllReports", () => {
416410
});
417411
});
418412

419-
// ---------------------------------------------------------------------------
420-
// fetchReportFiles
421-
// ---------------------------------------------------------------------------
422413
describe("fetchReportFiles", () => {
423414
afterEach(() => {
424415
globalThis.fetch = originalFetch;
@@ -623,9 +614,6 @@ describe("fetchReportFiles", () => {
623614
});
624615
});
625616

626-
// ---------------------------------------------------------------------------
627-
// fetchReportFileData
628-
// ---------------------------------------------------------------------------
629617
describe("fetchReportFileData", () => {
630618
afterEach(() => {
631619
globalThis.fetch = originalFetch;
@@ -841,9 +829,6 @@ describe("fetchReportFileData", () => {
841829
});
842830
});
843831

844-
// ---------------------------------------------------------------------------
845-
// renderMarkdownForTerminal
846-
// ---------------------------------------------------------------------------
847832
describe("renderMarkdownForTerminal", () => {
848833
test("returns empty string for empty input", () => {
849834
expect(renderMarkdownForTerminal("")).toBe("");
@@ -923,9 +908,6 @@ describe("renderMarkdownForTerminal", () => {
923908
});
924909
});
925910

926-
// ---------------------------------------------------------------------------
927-
// parseFlexibleDate
928-
// ---------------------------------------------------------------------------
929911
describe("parseFlexibleDate", () => {
930912
test("parses YYYY-MM-DD", () => {
931913
expect(parseFlexibleDate("2025-01-15")).toBe("2025-01-15T00:00:00.000Z");

monitoring_flask_backend/test_app.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,12 +1877,8 @@ def test_statement_timeout_returns_empty_metrics(self, mock_connect, client):
18771877
assert 'pgwatch_query_info{' not in data
18781878

18791879

1880-
# ---------------------------------------------------------------------------
1881-
# Additional coverage for /execute-query (issues 4, 6, 7, 8)
1882-
# ---------------------------------------------------------------------------
1883-
18841880
class TestExecuteQuerySuccessPath:
1885-
"""Issue 4 — success-path test: valid SELECT returning real results."""
1881+
"""Success-path test: valid SELECT returning real results."""
18861882

18871883
def test_returns_columns_and_rows(self, client, debug_mode):
18881884
"""A valid SELECT query returns 200 with populated columns and rows."""
@@ -1905,7 +1901,7 @@ def test_returns_columns_and_rows(self, client, debug_mode):
19051901

19061902

19071903
class TestExecuteQueryAuthSchemes:
1908-
"""Issue 6 — non-Bearer auth schemes (Basic, Token) must be rejected."""
1904+
"""Non-Bearer auth schemes (Basic, Token) must be rejected."""
19091905

19101906
def test_basic_auth_rejected(self, client, debug_mode):
19111907
"""Authorization: Basic <token> is not accepted (only Bearer is valid)."""
@@ -1941,7 +1937,7 @@ def test_apikey_scheme_rejected(self, client, debug_mode):
19411937

19421938

19431939
class TestExecuteQueryMultipleBlockComments:
1944-
"""Issue 7 — multiple sequential leading block comments before SELECT."""
1940+
"""Multiple sequential leading block comments before SELECT."""
19451941

19461942
def test_two_sequential_block_comments_accepted(self, client, debug_mode):
19471943
"""/* a */ /* b */ SELECT 1 passes the allowlist (both comments stripped)."""
@@ -1979,7 +1975,7 @@ def test_three_sequential_block_comments_accepted(self, client, debug_mode):
19791975

19801976

19811977
class TestExecuteQueryStatementTimeout:
1982-
"""Issue 8 — statement timeout during query execution returns 500."""
1978+
"""Statement timeout during query execution returns 500."""
19831979

19841980
def test_statement_timeout_returns_500(self, client, debug_mode):
19851981
"""OperationalError from statement_timeout inside execute-query returns 500."""
@@ -2001,7 +1997,7 @@ def test_statement_timeout_returns_500(self, client, debug_mode):
20011997
assert 'error' in data
20021998

20031999
def test_line_comment_containing_blocked_keyword_not_blocked(self, client, debug_mode):
2004-
"""Issue 2 regression: '-- pg_read_file' in a line comment must not trigger blocklist."""
2000+
"""Regression: '-- pg_read_file' in a line comment must not trigger blocklist."""
20052001
with patch('app.psycopg2.connect') as mock_connect:
20062002
mock_cursor = MagicMock()
20072003
mock_cursor.fetchall.return_value = [(1,)]

quality/scripts/release-readiness.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
# shellcheck shell=bash
33
#
44
# Release Readiness Check
5-
# TODO: add bats tests for this script
65
#
76
# Verifies that the codebase is ready for release by checking:
87
# 1. Git status (clean working tree, release-eligible branch)

reporter/postgres_reports.py

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -346,24 +346,20 @@ def get_queryid_queries_from_sink(self, query_text_limit: int = 655360, db_names
346346
"""
347347
cursor.execute(query)
348348

349-
# Use iterator to fetch rows in batches instead of loading all at once
350349
for row in cursor:
351350
db_name = row['dbname']
352351
queryid = row['queryid']
353352
query_text = row['query']
354-
355-
# Skip if queryid is missing
353+
356354
if not queryid:
357355
continue
358-
359-
# Truncate query text if it exceeds the limit
356+
360357
if query_text and len(query_text) > query_text_limit:
361358
query_text = query_text[:query_text_limit] + '...'
362-
363-
# Initialize database dict if needed
359+
364360
if db_name not in queries_by_db:
365361
queries_by_db[db_name] = {}
366-
362+
367363
queries_by_db[db_name][queryid] = query_text or ''
368364

369365
except Exception as e:
@@ -639,22 +635,9 @@ def generate_a007_altered_settings_report(self, cluster: str = "local", node_nam
639635

640636
return self.format_report_data("A007", altered_settings, node_name, postgres_version=self._get_postgres_version_info(cluster, node_name))
641637

642-
# ==================================================================================
643-
# H001: Invalid Indexes - Observation Data for Decision Tree
644-
# ==================================================================================
645-
#
646-
# This report collects observation data that enables decision tree analysis:
647-
# - has_valid_duplicate / valid_duplicate_name: Is there a valid index on same column(s)?
648-
# - is_pk / is_unique / constraint_name: Does it back a constraint (UNIQUE / PK)?
649-
# - table_row_estimate: Is the table small (< 10K rows)?
650-
#
651-
# The JSON report contains ONLY observations (raw data).
652-
# Recommendations (DROP, RECREATE, UNCERTAIN) are computed at render time:
653-
# - CLI: cli/lib/checkup.ts -> getInvalidIndexRecommendation()
654-
# - UI: Grafana dashboard or web template applies the same logic
655-
#
656-
# ==================================================================================
657-
638+
# H001 collects observation data only; recommendations (DROP, RECREATE, UNCERTAIN)
639+
# are computed at render time by CLI (cli/lib/checkup.ts -> getInvalidIndexRecommendation())
640+
# and UI (Grafana / web template) using the same decision tree.
658641
def generate_h001_invalid_indexes_report(self, cluster: str = "local", node_name: str = "node-01") -> Dict[
659642
str, Any]:
660643
"""

0 commit comments

Comments
 (0)