fix: replace console.log with structured logging in non-CLI-output code#765
fix: replace console.log with structured logging in non-CLI-output code#765carlos-alm merged 6 commits intomainfrom
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review: Replace console.log with structured logging
|
Greptile SummaryThis PR moves Confidence Score: 5/5Safe to merge — clean refactor with no logic changes, all imports updated, and prior review concern addressed. All changes are correct separations of concern with no behaviour change. The only previously-flagged issue (missing clarifying comment on intentional console.log) has been resolved. No P0 or P1 findings remain. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["outputResult(data, field, opts)"] -->|opts.ndjson && field === null| B["console.log(JSON.stringify(data))"]
A -->|opts.ndjson && field !== null| C["printNdjson(data, field)"]
C -->|data._pagination exists| D["console.log(JSON.stringify({_meta: ...}))"]
C -->|items array| E["for each item: console.log(JSON.stringify(item))"]
subgraph "presentation/result-formatter.ts (after PR)"
A
C
D
E
B
end
subgraph "shared/paginate.ts (after PR)"
F["paginate()"]
G["paginateResult()"]
end
H["tests/integration/pagination.test.ts"] -->|import printNdjson| C
I["features/ast.ts"] -->|process.stdout.write| J["stdout"]
Reviews (2): Last reviewed commit: "fix: add clarifying comment for intentio..." | Re-trigger Greptile |
| if (data._pagination) console.log(JSON.stringify({ _meta: data._pagination })); | ||
| const items = data[field]; | ||
| if (Array.isArray(items)) { | ||
| for (const item of items) console.log(JSON.stringify(item)); | ||
| } |
There was a problem hiding this comment.
printNdjson still uses console.log after relocation
The PR description frames these changes as fixing structuredLogging (Rule 15) violations, but printNdjson still uses console.log in its new home in result-formatter.ts. The actual fix here is correct separation of concerns — removing I/O from a pure-utility module (paginate.ts) and placing it in a presentation module where output is expected. Using console.log in result-formatter.ts is intentional and appropriate for NDJSON output to stdout, but the commit message and PR summary may create confusion if someone later audits remaining console.log calls looking for logging violations — they'll find this and wonder why it wasn't converted.
A brief inline comment explaining that console.log is intentional here (stdout serialisation, not debug logging) would pre-empt that confusion:
| if (data._pagination) console.log(JSON.stringify({ _meta: data._pagination })); | |
| const items = data[field]; | |
| if (Array.isArray(items)) { | |
| for (const item of items) console.log(JSON.stringify(item)); | |
| } | |
| if (data._pagination) console.log(JSON.stringify({ _meta: data._pagination })); | |
| const items = data[field]; | |
| if (Array.isArray(items)) { | |
| // console.log is intentional: NDJSON serialisation must go to stdout, not the structured logger | |
| for (const item of items) console.log(JSON.stringify(item)); | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Done -- added the clarifying comment in 43b4ebc. The inline comment now explains that console.log is intentional NDJSON serialisation to stdout, not debug logging.
There was a problem hiding this comment.
Done -- added the clarifying comment in 43b4ebc. The inline comment now explains that console.log is intentional NDJSON serialisation to stdout, not debug logging.

Summary
console.logwithlogger.info/logger.warnin non-CLI-output codepaginate.ts,ast.ts, andresult-formatter.tsTitan Audit Context
Changes
src/shared/paginate.ts- Replace console.log with logger callssrc/features/ast.ts- Replace console.log with logger callssrc/presentation/result-formatter.ts- Replace console.log with logger callstests/integration/pagination.test.ts- Update test assertionstests/presentation/result-formatter.test.ts- Update test assertionsTest plan