refactor: extract format layer, dedup helpers, precompile keyword regexes#24
Merged
Conversation
The severity formula's thread-depth term was dead in production: thread bodies were never fetched, so thread_count was always 0 and min(0*10, 50) contributed nothing. Use the list API's threads count field instead (no extra API calls) and treat 0/missing as the baseline 1. Also: - Clamp recency so an unparseable created_at can't produce NaN scores and future-dated tickets can't pin severity to 100 - Strip PII-redaction placeholders before n-gram tokenization so 'email redacted' can't surface as an emerging theme - Delete the unreachable thread-body fetch path (includeThreads was never set by any caller; last present at d5f985f) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- One failing ProductLift portal no longer drops every portal's data; failures surface as named-portal warnings in the response - Total fetch failure (all sources) returns isError instead of an empty analysis that reads as success, and is never cached - Replace the module-global PII category log with an explicit per-request sink; concurrent tool calls no longer interleave audit metadata - get_feature_requests reports pii_scrubbing_applied and pii_categories_redacted, and stops sending commenter names (role only) - A malformed PRODUCTLIFT_PORTALS degrades to zero portals with the error surfaced in tool descriptions instead of killing the server - Label feature requests with their actual source portal - Guard themes.config.json loading with an actionable error - Scrub upstream error text in warnings; collapse whitespace in quotes - preview_only now describes exactly what is fetched and sent Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…exes Pure move plus dedup; no behavior change. analyzeFeedback output verified byte-for-byte identical on a fixed dataset across this change. - New src/format.ts holds the PII-scrubbing formatters, quote extraction, agent-response detection, and detail trimming (index.ts is untestable by import: it exits without env and connects a transport at load) - New helpers replace duplicated blocks in index.ts: toErrorResult (4 identical catch bodies), buildLookupMaps, signalTypeOf, capTitles; get_feature_requests reuses filterClientsByPortal - Theme keyword regexes compile once per analyzeFeedback run instead of per data point x theme x keyword; matchesTheme keeps its signature and semantics (multi-word substring, word-boundary singles, escaping, no g flag) - New guardrail tests for parsePortalConfigs env parsing and the format layer (24 new tests; 64 total) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Transitive dep of @modelcontextprotocol/sdk; npm audit flagged the 4.12.x line high (GHSA-xrhx-7g5j-rcj5 and related). Lockfile-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #22, which GitHub auto-closed when its base branch (#21's) was deleted on merge. Same content, now based on main. Pure refactor; should review as moved code with zero behavior change.
src/format.ts: the PII-scrubbing formatters, quote extraction, agent-response detection, and detail trimming.index.tscan't be imported in tests (it exits without env vars and connects a stdio transport at module load), so the testable format layer moved out to get coverage.index.ts: onetoErrorResultreplaces 4 identical catch bodies,buildLookupMaps/signalTypeOf/capTitlesreplace the twice-written lookup-map, signal-type, and title-capping blocks, andget_feature_requestsreusesfilterClientsByPortalinstead of its inline copy.analyzeFeedbackrun instead of once per data point per theme per keyword. Semantics preserved exactly: multi-word keywords stay substring matches, single-word keywords stay word-boundary regexes with escaping, nogflag, per-run compilation sothemes.config.jsonstays hot-editable.parsePortalConfigsenv parsing and the format layer (thread-count mapping, email redaction, PII sink collection, no commenter names, detail-level shapes, title capping).Verification: build and 64 tests green;
analyzeFeedbackoutput compared byte-for-byte on a fixed synthetic dataset across the refactor — identical. The merge commit from main resolves with theoursstrategy because main's squash of #21 is content-identical to this branch's own #21 ancestry (tree diff: 0 lines).🤖 Generated with Claude Code