Conversation
…ing, loopback-default HOST - listSpecs query (LEFT JOIN paragraph counts) exposed as GET /specs - getSpecTree (populated tree + cross-references) exposed as GET /specs/:id/tree (previously REST returned parts:[] always; the populated tree was MCP-only) - express.static(public/) mounted ahead of the router for the demo SPA - HOST env (default 127.0.0.1) so the unauthenticated demo server is never LAN-exposed by accident; opt-in via HOST=0.0.0.0
Drafting-room blueprint UI served from public/ (vanilla ES modules, no build): - drag-and-drop multi-file ingest via POST /parse with job polling, per-file stage progress, and 429 rate-limit backoff - CSI hierarchy sheets: faithful getLabel port (PART N - / N.N / A. / 1. / a.), collapsible parts/articles, [NOTE] callouts, vanish tags - cross-reference web: SVG arc diagram of section-to-section citations with loaded / in-library / unresolved states; client-side resolution by section number so links heal regardless of upload order - in-text Section XX XX XX linkification (notes included) with jump-and-flash navigation; per-sheet citation chips and collapsible standards list
Start-SpecR.bat -> scripts/windows/Start-SpecR.ps1 (PowerShell 5.1, ASCII-only): - portable Node.js 22 + pnpm + PostgreSQL 16 into .specr-runtime/ — no admin rights, no system installs, removable by deleting one folder - initdb (trust auth, loopback-only) + pg_ctl on port 5439, reused across runs - pnpm install --frozen-lockfile, migrate, seed, build, then node dist/index.js - pre-checks the app port, opens the browser only once the server answers - overrides: SPECR_PORT / SPECR_PG_PORT / SPECR_DATABASE_URL / *_VERSION - .gitattributes pins CRLF for .bat/.ps1 and exempts UFGS latin1 fixtures
Double-clicking on a locked-down machine hit PSSecurityException /
UnauthorizedAccess ('running scripts is disabled on this system'):
- preflight (best-effort): Unblock-File clears Mark-of-the-Web from zip
extracts, and Set-ExecutionPolicy -Scope CurrentUser RemoteSigned
registers the standing exception so direct .ps1 runs work too
- launch via Invoke-Expression of the script TEXT: command text is not a
script file, so execution policy -- including GPO-enforced
Restricted/AllSigned that ignores -ExecutionPolicy Bypass -- cannot
block it
- repo path travels via SPECR_REPO_ROOT env (never interpolated into PS
string literals), so spaces, parens, and apostrophes in the path are
inert; PS1 falls back to it when $PSScriptRoot is empty under IEX
- explicit exit-code propagation (required in -Command mode) so the
bat's errorlevel check still surfaces failures
Every installer step is now visible in the cmd window: - Invoke-Native takes a step label (>> prefix) and streams the tool's own stdout/stderr line by line (initdb, pg_ctl, createdb, npm) instead of discarding it - Get-Download rewritten as a manual stream copy with a live percent / MB counter, so the ~320 MB PostgreSQL archive never looks frozen - extraction, reuse-of-cached-artifact, env settings, and resolved tool paths (node/pnpm locations, npm global prefix) are all echoed pnpm install failure (reported: 'could not install pnpm') hardened: - npm's full output now streams (dropped --loglevel=error) so the real cause is visible - $global:LASTEXITCODE = 9009 sentinel in Invoke-Native -- a missing executable previously left a stale exit code that could read as success or mask the real failure - after install, pnpm.cmd is resolved via 'npm prefix --global' and that dir is prepended to PATH (fresh installs often don't have %APPDATA%\npm on the session PATH yet) - throw messages now include the npm exit code, likely causes (proxy / registry block / antivirus), and a manual workaround
…adlock) A real run stalled right after pg_ctl printed 'server started' -- the demo's HTTP server never came up. Root cause: pg_ctl start spawns the postgres daemon, which inherits duplicates of the console pipe handles that Invoke-Native's capture pipeline (2>&1 | ForEach-Object) reads. pg_ctl exits, but the pipe never reaches EOF while postgres holds the write handle, so the script blocked forever before the psql check, dependency install, build, and node startup. - new Invoke-NativeUnpiped: Start-Process -NoNewWindow -PassThru + Process.WaitForExit() -- no pipes, so no EOF dependency; waits only on pg_ctl itself, never the postgres descendant (Start-Process -Wait on PS 5.1 waits for the whole tree and would re-hang) - pg_ctl start routed through it, args as a single pre-quoted string (space-safe paths, -o "-p PORT" reaches postgres as one token) - clarifying console line: 'server started' is PostgreSQL; the SpecR web server follows the build steps
…errupted runs Field report: pg_ctl start 'failed' per the exit-code check while postgres.log ended with 'database system is ready to accept connections' -- the server was fine, the exit code lied (PS 5.1 Start-Process -PassThru can hand back a Process whose ExitCode reads null, and null -ne 0 is true). - Invoke-NativeUnpiped now uses raw [System.Diagnostics.Process]::Start (UseShellExecute=false): still no pipes (no daemon EOF deadlock), WaitForExit on the launcher only, and a reliable non-null ExitCode - readiness is now decided by the server itself: poll pg_isready for up to 30s after pg_ctl start; proceed (with a note) when the server accepts connections even if pg_ctl's exit code claimed failure; only fail when the probe never succeeds - on real failure, print the last 40 lines of postgres.log inline plus targeted hints (missing VC++ redistributable when no log exists, port conflicts, antivirus, elevation) instead of 'see the log' - self-heal interrupted runs: stop orphaned postgres.exe processes from OUR runtime dir (they hold the port + shmem), bump to the next free port past foreign listeners, and when the server is already running read its actual port from postmaster.pid line 4 - proactive warnings: vcruntime140.dll missing, elevated console
…ERT_LOCALLY) Behind a TLS-inspecting proxy or antivirus, the corporate root CA lives in the Windows certificate store -- which PowerShell uses (our direct downloads worked) but Node ignores in favor of its bundled CA list. So npm/pnpm fail every registry request with UNABLE_TO_GET_ISSUER_CERT_LOCALLY and crawl through retry backoff, which is also why pnpm install was so slow. Bootstrap now exports LocalMachine/CurrentUser Root+CA stores to .specr-runtime/windows-ca-bundle.pem and sets NODE_EXTRA_CA_CERTS for the session before any npm/pnpm/node call -- TLS verification stays ON, Node just gains the same trust roots the OS already has. Opt out with SPECR_NO_SYSTEM_CA=1.
…d notes properly tree.parts holds ROOT nodes, not necessarily part-type nodes — ARCAT preamble (title lines, copyright, specifier note) lands at root by design. The sheet renderer labelled every root by raw array index, so preamble scraps displayed as 'PART 1..10' and the real parts became PART 11/12/13 with articles numbered 11.1. - PART numbering now counts only part-type nodes (articles correctly read 1.1 again) - root-level notes render as amber note callouts - remaining root continuations render as dim italic front-matter lines - empty-text roots from pre-fix DB rows are skipped defensively
Footer section chips previously only jumped to the referenced sheet. Each chip is now a split control: - main button walks the citation sites of that section WITHIN this spec's body — cycling on repeated clicks with a position counter (2/3), auto-expanding collapsed parts/articles on the way, and flashing a locate ring on the found link - the new arrow tail keeps the jump-to-referenced-sheet behavior (loaded -> jump; in-library -> load hint; unresolved -> walk only) In-body links still hop to the target sheet, so citations can be ping-ponged: walk them here, jump across, walk the next spec's.
Hovering any in-body section reference shows a floating < n/N > pill: - next/prev step through ALL citation sites in that spec's body in document order, wrapping at the ends - each step locate-flashes the link, auto-expands collapsed parts/articles on the way, and smooth-scrolls it to center - the pill is position:fixed and deliberately stays put under the cursor while the page scrolls beneath it, so next-next-next walks the whole spec without chasing the control - grace-period hide on mouse-out; dropped when a citation link is clicked (cross-sheet navigation takes over) locateLink/expandAncestors extracted from the chip walker and shared.
Proprietary files can't be shared for diagnosis, so failures must be self-explanatory locally: - failed queue items show [HTTP status / job id] before the server's full error message, plus a hint that the complete response is in the browser console - uploadSpec/waitForParseJob log the full server response / job object via console.error and attach status/jobId to thrown errors
Field report: walking references scrolled correctly but never visibly highlighted the in-body citation. Root cause: the 1.5s locate pulse started at CLICK time while smooth scrollIntoView was still in flight — on these sheets the target is often thousands of pixels away, so the animation completed before the link ever entered the viewport. - persistent is-current marker (orange outline) applied immediately and moved on each step, so the found citation stays identifiable - the pulse now fires via IntersectionObserver when the link actually arrives in view, regardless of scroll distance; observer disconnects after firing (10s hygiene timeout otherwise) - pulse made bolder (double ring) now that it fires where you're looking
Field report: a specific spec listed a reference in the footer but
clicking it could not snap to the citation in the body. Root cause:
the ingest extractor matches 'Section \d\d\s+\d\d\s+\d\d' — tolerant of
non-breaking spaces and doubled spaces, both routine in Word text — and
stores a normalized targetSection, while the SPA linkifier demanded
single ASCII spaces. The ref row existed; the body link was never
created; the chip had nothing to walk to.
- SECTION_PATTERN now tolerates \s{1,3} between digit groups (covers
NBSP); matched text is displayed verbatim but normalized into
link.dataset.section for status lookup, navigation, and walking
- walkToCitation matches on dataset.section instead of textContent
- when a walk still finds nothing, the chip no longer fails silently:
a toast says whether the section number appears in the sheet text at
all, and the console gets specifics for reporting
Reproduced with a synthetic DOCX carrying both NBSP and double-space
citations: pre-fix zero body links; post-fix both linkify, walk 1/1,
and snap with the is-current marker.
…ports merge semantics)
…lings (#122) Port of the generator/markdown.ts fix to the mockup renderer: number a node's children by an ordinal that only advances past numbered siblings, so specifier-note banners no longer shift the list (1..15 rendered 5..20). New consumesNumber/appendNumberedChildren helpers; getLabel unchanged. Refs #122
Two demonstrable edit features on the linkage-console SPA, showcasing the backend's deterministic cross-reference integrity (not for shipping). Feature A — delete a paragraph: a hover ✕ on each paragraph removes it; the spec_references ON DELETE CASCADE drops the contained citation in one statement. The confirm dialog counts references across the whole deleted subtree. Feature B — inline editor: click a paragraph to edit its text; on save, the removed citations are detected by per-section occurrence counts, and a modal offers Cancel / delete-the-reference / delete-reference-and-remove-spec-from- project. The board is backed by one hidden demo project so removeSpecFromProject's broken-ref cascade spans the corpus; the masthead shows a server-computed BROKEN REFS count, and re-dropping a removed section heals the broken links. Backend (src/, CI-checked): - getSpecTree references now expose id + sourceParagraphId - new DB mutations: deleteParagraph, updateParagraphText, deleteReference, deleteSpec - new routes: DELETE /specs/:id, DELETE+PATCH /specs/:id/paragraphs/:pid, DELETE /specs/:id/references/:rid (UpdateParagraphBodySchema, shared pg-error helper) - removeSpecFromProject cascade also breaks UNRESOLVED section-only citations, gated on target_spec_id IS NULL so a citation resolved to a surviving same-section spec (same section, other source) is never collateral-damaged Frontend (public/, not CI-linted): - api.js mutation + project client; modal.js (promise-based confirm/choice); tree.js per-paragraph ✎/✕ + textarea editor + broken-ref styling; app.js demo-project bootstrap, mutation orchestration, broken-ref counter - self-citations and multi-source duplicate sections handled so an edit can never delete the spec being edited and removes every loaded same-section sheet Tests: deleteParagraph cascade, updateParagraphText, deleteReference, deleteSpec (409 on project pin), and the multi-source cascade regression. Full suite green (601 unit, 258 integration); verified end-to-end in the browser.
* fix(parser): tolerate dirty SEC metadata Sanitize XML-unsafe legacy control characters before SEC XML parsing while still rejecting null bytes. Allow SEC files without SCN metadata to parse as unknown so the existing inference stage can recover from document content. Co-Authored-By: Claude <noreply@anthropic.com> * fix(parser): decode SEC section entities before prefix strip Decode SCN entity references before removing the SECTION prefix so entity whitespace still normalizes to canonical section numbers. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Support SCN and STL metadata nested inside SEC heading wrappers such as HL3, covering UFGS 11_72_13 without weakening the missing-title validation. Co-authored-by: Claude <noreply@anthropic.com>
Bring 34 commits of main's backend features into the mockup demo branch so the new APIs (document concurrency + advisory locks, editability/conventions, revision nomenclature, manual assembly + front matter, SEC output renderer, source facts, spec diff/merge, deep paragraph nesting, header/footer config, Scalar API docs) can be exercised through the linkage-console UI. Conflict resolutions (14 files): - parser/sec: take main's pr5/pr6/pr7 deep-nesting depth map (#215) over mockup's pr5 cap; mockup's SEC metadata fixes (#173/#175) already on main. - api/paragraphs + db/queries/paragraphs: keep mockup's deleteParagraph; adopt main's gated, optimistic-concurrency updateParagraphText (UpdateParagraphResult). - api/router: union of mockup demo routes (list/tree/delete spec, delete/edit paragraph, references, coordination, required-sections, libraries) and main's new routes (locks, style-source, diff, merge, manual/revision generate); parse stays unthrottled for stakeholder demos. - ast schemas/index: drop mockup's duplicate CreateRevision/UpdateParagraph (main owns them via revision-schemas.js + the expectedVersion superset); keep SetRequiredSections; restore the missing SectionNumberFormatSchema import. - mcp/tools: register both coordination_report (mockup) and get_spec_diff (main). - openapi: split the interleaved PATCH /projects/{id} and POST /projects/{id}/generate into two complete operations. - .gitignore: narrow main's public/ ignore to public/scalar/ so the committed mockup SPA stays tracked. Migration renumbering: revert division_general_specs to its canonical 022 (byte-identical to main), move mockup's new migrations after main's highest — 022_create_required_sections → 031, 024_project_settings → 032. Clean 001–032, no duplicate prefixes. Line-cap (max-lines 400) restoration after the merge concatenated both branches' additions: extract handleCoordinationReport → src/mcp/coordination-handler.ts and getBrokenRefs/BrokenRef → src/db/queries/broken-refs.ts. Verified: pnpm build, pnpm lint (eslint + tsc --noEmit + prettier), and pnpm test (1004 unit tests) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The main merge brought constraints and handler behavior that mockup's pre-existing integration fixtures predated. None of these reflect a product defect — the real parse/persist flow already satisfies every constraint; only the raw-INSERT test fixtures bypassed them. - specs_owner_xor (migration 016): demo-mutation fixtures inserted bare specs with no owner. Provide library_id (matching the pattern main applied to its own fixtures): UFGS Reference, or distinct libraries for the multi-source cascade test. Fixes mutations/edit-mutations/cascade-multisource suites. - PATCH /specs/:id/paragraphs adopts main's gated handler: z.uuid()-strict params (synthetic all-zero-version ids → valid v4) and 400 (not 422) on empty text; assert main's contract. - listProjects now carries sectionNumberFormat (ProjectListItem): assert with objectContaining in the refs + MCP list_projects tests. - GET /specs/:id/tree: testSpecId is seeded with a baseline part in the file's top beforeAll, so find this block's 'GENERAL' part by text, not by index. Integration: 620/623 pass. Remaining: the openapi route↔spec contract gate (mockup's demo routes are undocumented — pending a doc-vs-exclude decision) and cpi.integration (parser byte-identical to main; local copyrighted fixture lacks the ART/PRT styles detectSource keys on — CI skips it as fixtures are gitignored). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 8 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a complete ChangesSpecR Web UI Demo
Sequence Diagram(s)sequenceDiagram
rect rgba(30, 80, 140, 0.5)
Note over User,CoordJs: Demo Bootstrap
participant User
participant StartScript as Start-SpecR.sh/.ps1
participant ApiProcess as SpecR API (port 3000)
participant ServerMjs as server.mjs (port 3001)
User->>StartScript: run launcher
StartScript->>ApiProcess: spawn (pnpm install → migrate → seed → build → node dist/index.js)
StartScript->>ServerMjs: node server.mjs (SPECR_API_BASE=http://127.0.0.1:3000)
end
rect rgba(80, 40, 120, 0.5)
Note over Browser,CoordJs: SPA Init and File Ingest
participant Browser
participant AppJs as app.js
participant ApiJs as api.js
participant Dropzone as dropzone.js
participant TreeJs as tree.js
participant WebJs as web.js
participant CoordJs as coordination.js
Browser->>ServerMjs: GET /index.html
ServerMjs-->>Browser: static HTML/CSS/JS
Browser->>AppJs: DOMContentLoaded bootstrap
AppJs->>ApiJs: checkHealth(), listLibraries(), listProjects()
AppJs->>ApiJs: getRequiredSections(projectId)
AppJs->>Dropzone: initDropzone({ onSpecReady })
User->>Dropzone: drop .sec/.docx files
Dropzone->>ServerMjs: POST /parse (FormData)
ServerMjs->>ApiProcess: proxy multipart upload
ApiProcess-->>ServerMjs: { jobId }
ServerMjs-->>Dropzone: { jobId }
Dropzone->>ApiJs: waitForParseJob(jobId, onProgress)
ApiJs-->>Dropzone: job.result (complete)
Dropzone->>AppJs: onSpecReady(spec)
AppJs->>ApiJs: getSpecTree(specId)
AppJs->>TreeJs: renderSpecSheet(spec, sheetCtx)
AppJs->>WebJs: buildWebModel(specs) → renderWeb(canvas, model)
AppJs->>ApiJs: getBrokenRefs(projectId)
AppJs->>ApiJs: getCoordinationReport(projectId)
AppJs->>CoordJs: renderCoordinationReport(container, report, ctx)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
examples/web_ui_demo/Start-SpecR.sh (1)
30-38: ⚡ Quick winWait for API readiness before launching the demo server.
Line 30 starts the API and Line 38 immediately starts the demo; this can race and surface avoidable “API unavailable” failures during boot.
Suggested patch
node dist/index.js & API_PID="$!" + +ready=0 +for _ in {1..60}; do + if curl -fsS "http://127.0.0.1:$API_PORT/health" >/dev/null; then + ready=1 + break + fi + sleep 0.5 +done +if [[ "$ready" -ne 1 ]]; then + printf 'SpecR API did not become ready on port %s\n' "$API_PORT" >&2 + exit 1 +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/web_ui_demo/Start-SpecR.sh` around lines 30 - 38, The Start-SpecR.sh script starts the API process with `node dist/index.js &` and assigns the PID to API_PID, but immediately launches the demo server with `node server.mjs` without waiting for the API to be ready, causing potential race conditions. After the API_PID assignment and before the cd command that precedes launching the demo server, add a polling loop that checks if the API is ready to accept requests (you can use curl or similar to probe the API port specified by API_PORT) with a reasonable timeout and retry count to ensure the API is listening before proceeding to start the demo server.examples/web_ui_demo/scripts/windows/Start-SpecR.ps1 (1)
43-56: ⚡ Quick winAdd an API health wait loop before opening the demo.
After Line 43, the script launches the demo flow immediately; if the API is still booting, early calls can fail.
Suggested patch
$api = Start-Process -FilePath 'node' -ArgumentList 'dist/index.js' -WorkingDirectory $RepoRoot -NoNewWindow -PassThru + +$ready = $false +for ($i = 0; $i -lt 60; $i++) { + try { + $resp = Invoke-WebRequest -Uri "http://127.0.0.1:$ApiPort/health" -UseBasicParsing -TimeoutSec 2 + if ($resp.StatusCode -eq 200) { $ready = $true; break } + } catch {} + Start-Sleep -Milliseconds 500 +} +if (-not $ready) { + throw "SpecR API did not become ready on port $ApiPort" +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/web_ui_demo/scripts/windows/Start-SpecR.ps1` around lines 43 - 56, The script starts the API process with Start-Process but immediately proceeds to display connection information and launch the web UI without verifying the API is ready to accept requests. Add a health check wait loop after the Start-Process line that launches the API node process. This loop should poll the API endpoint at http://127.0.0.1:$ApiPort with a reasonable timeout and retry interval to confirm the API is responding before allowing the script to continue with the Write-Host statements and the Start-Process call that opens the web UI demo.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/web_ui_demo/css/app.css`:
- Line 26: The stylelint value-keyword-case rule is failing on the font-family
declarations at lines 26 and 1546. CSS generic font family keywords (such as
serif, sans-serif, and monospace) must be lowercase according to stylelint's
default configuration. Review both the --serif custom property declaration and
the similar declaration at line 1546, and ensure all keyword values within the
font-family lists are in lowercase, particularly the generic font family
keywords at the end of each declaration.
In `@examples/web_ui_demo/js/api.js`:
- Around line 79-82: The console.error() call in the SpecR upload rejection
error handler violates the no-console lint rule for non-test/non-script
JavaScript files. Remove the console.error() statement and instead attach the
diagnostic information to the error object being thrown (the err variable),
allowing the UI layer to decide where and how to display the error. Apply this
same refactoring to the second console.error() occurrence mentioned at lines
171-173 in the same file.
In `@examples/web_ui_demo/js/app.js`:
- Line 440: The console.warn call at line 440 in the app.js file (and similar
console calls at lines 473, 1453, 1465, and 1552) violate the no-console linting
policy enforced for production JavaScript modules. Remove all console.* method
calls (console.warn, console.log, console.error) from these locations in the
app.js file since logging to the console is not allowed in production code
according to the project's linting configuration.
- Around line 1-1778: The app.js file at 1778 lines exceeds the 400-line maximum
specified in the coding guidelines and should be split into separate modules
organized by functional concern. Create the following new module files: a
project manager module containing functions like activeProjectName,
activeSectionNumberFormat, refreshProjectList, switchProject,
saveProjectSettings, loadActiveProjectWorkspace, and initProjectManager; a TOC
builder module containing makeNode, loadedTocEntries, compareTocSections,
groupedTocSections, renderTocBuilder, renderTocDivisionGroup, addTocCandidate,
matchingTocSpecs, renderTocCandidates, refreshTocBuilder, saveTocBuilder, and
initTocBuilder; a library manager module containing selectedLibrary,
companyMaster, clientLibraries, renderLibraryView, selectLibrary,
renderLibraryDetail, renderLibraryTree, addClientLibrary, renameSelectedClient,
removeSpecFromLibrary, and initLibraryManager; a mutations module containing
onDeleteParagraph, onSaveParagraphEdit, commitTextEdit, removeTargetSpecs,
onRemoveSpecFromProject, and onAddSpecToToc; and a utilities module for helper
functions like toast, preview, and subtreeNodeIds. Update app.js to import these
modules and call their initialization functions, keeping only the core state
management, boot sequence, and sheetCtx definition in the main file.
- Around line 1426-1439: The commitTextEdit function calls updateParagraph
before the deleteReference operations in a loop, which means if any delete
fails, the paragraph update has already been persisted to the server while the
catch block returns "cancelled" suggesting the operation failed completely. To
mitigate this UI/state consistency issue, ensure that after catching an error in
the catch block, the app reloads all specs from the server using reloadAllSpecs
and refreshes the board state with renderBoard to reflect the actual persisted
state on the server, so the UI accurately reflects what operations succeeded and
which failed rather than leaving inconsistent state.
In `@examples/web_ui_demo/js/dropzone.js`:
- Around line 77-233: The initDropzone function exceeds the
max-lines-per-function limit of 50 lines and needs to be refactored into smaller
functions. Extract the logical blocks into separate internal functions: create
one function for initializing all DOM event listeners (for dragenter, dragleave,
dragover, drop events), another for setting up button click handlers (pickBtn
and addFab), another for the file input change handler, and consider moving the
nested functions like makeItem, setStage, fail, processOne, pump, accept, and
chooseFiles to be module-level or better organized to reduce the main
initDropzone body to under 50 lines. Ensure initDropzone still maintains its
public interface and calls these extracted functions to preserve the existing
behavior.
In `@examples/web_ui_demo/js/modal.js`:
- Around line 55-61: The current focus trapping in the onKey event handler only
restricts Tab navigation to action buttons via trapFocus(event, buttons), which
leaves body inputs unreachable and allows focus to escape the modal in
openPicker. Expand the focus trapping implementation to collect all tabbable
elements throughout the entire modal (including both buttons and input fields),
pass the complete tabbable elements list to trapFocus instead of just buttons,
and set initial focus to the first tabbable element when each modal variant
(openChoice and openPicker) opens. Ensure the trapFocus function logic properly
cycles focus between the first and last tabbable elements to prevent escape.
- Around line 34-35: The code directly removes the DOM element of the previous
modal (openVeil) without invoking its proper cleanup path, which leaves the
modal's promise unresolved and key listeners active. Instead of calling
openVeil.remove() directly, you need to trigger the prior modal's cleanup or
finish method to ensure its promise is resolved and all listeners are properly
unregistered before removing the DOM element. This applies to all instances
where openVeil is removed without going through the modal's completion/closure
flow (including the locations at lines 64-70, 95-96, and 156-162).
- Around line 32-82: The openChoice and openPicker functions exceed the 50-line
maximum enforced by the max-lines-per-function ESLint rule. Refactor both
functions by extracting helper functions to reduce their complexity. For
openChoice, extract the dialog element creation logic (el calls and setAttribute
calls), the button creation and event setup logic (the choices.map block), and
the keyboard event handler setup into separate helper functions like
createDialogElement, createChoiceButtons, and setupKeyboardHandlers. Apply
similar refactoring to openPicker to break down its structure. Ensure each
extracted function has a single responsibility and that both openChoice and
openPicker end up below 50 lines by delegating setup tasks to these helper
functions.
In `@examples/web_ui_demo/js/popover.js`:
- Around line 21-103: The initRefPopover function exceeds the 50-line maximum
limit at approximately 83 lines. Extract the event listener setup code into
separate helper functions to reduce the main function size. Specifically, create
separate functions to handle the event wiring for the prev and next button
clicks, the popover mouse events (mouseenter and mouseleave), and the
document-level mouseover and click event listeners. Keep the state variables
(anchor, hideTimer) and core helper functions (sheetLinks, updateCounter,
showAt, hide, scheduleHide, step) within the main function, but move all the
addEventListener calls to separate, well-named functions that are called from
initRefPopover to improve readability and comply with the size constraint.
In `@examples/web_ui_demo/js/tree.js`:
- Around line 1-540: The file tree.js exceeds the 400-line limit and contains
functions that violate the 50-line maximum. Refactor by extracting the citation
walking and footer rendering logic into separate modules: create a dedicated
module for citation walk functionality (containing `walkToCitation`,
`pulseOnArrival`, `locateLink`, and related helpers) and move the footer
rendering concerns (the `renderRefsFooter` and `makeSectionChip` functions which
handle reference display) into their own module. Similarly, extract the inline
edit and delete affordances (the `makeParaActions`, `beginEdit`, `commitEdit`,
and related helper functions like `autosize` and `removedReferences`) into a
separate editing module. This will reduce tree.js to core rendering concerns and
bring all functions, including `renderSpecSheet`, under their respective size
limits.
- Around line 363-368: Remove the console.warn statement that logs the SpecR
citation walk warning message in tree.js. The console.warn call violates the
no-console ESLint rule which is configured as an error for all JavaScript and
TypeScript files in the project. Delete the entire console.warn block including
the conditional message that checks for bodyHasPattern.
In `@examples/web_ui_demo/js/web.js`:
- Around line 75-168: The renderWeb function exceeds the maximum allowed 50
lines per function. Refactor by extracting the arc rendering logic (the loop
iterating over model.edges) into a separate helper function, and extract the
node rendering logic (the loop iterating over model.nodes) into another separate
helper function. Additionally, extract the position and dimension calculation
logic into a separate helper function. Keep the main renderWeb function as an
orchestrator that handles the empty state check, calls these helper functions,
and appends elements to the canvas.
- Around line 59-63: When an edge already exists in the edgeMap and you
increment the count, you must also update the status field to preserve the
broken precedence. In the if (existing) block, after incrementing
existing.count, update existing.status to ensure that if either the existing
edge or the new edge has a broken status, the aggregated edge's status reflects
that it is broken. This ensures that the broken state is never lost when
aggregating duplicate edges.
In `@examples/web_ui_demo/server.mjs`:
- Around line 44-47: The readRequestBody function buffers the entire incoming
request body without any size limit, which can cause memory exhaustion with
large uploads. Add a maximum buffer size limit to the function by tracking the
total bytes read as chunks are accumulated in the loop, and either stop reading
or return undefined once the limit is exceeded. This prevents unbounded memory
growth when processing large request bodies.
- Around line 82-83: The decodeURIComponent call on the line that assigns the
pathname variable executes outside of your try-catch block and can throw an
error when the URL contains invalid escape sequences, causing unhandled
exceptions. Wrap the decodeURIComponent call in a try-catch block to handle
malformed URL encoding gracefully, ensuring that invalid URLs return a
controlled error response to the client instead of crashing the request handler.
In `@examples/web_ui_demo/Start-SpecR.bat`:
- Around line 5-9: The issue is that the `pause` command on line 8 overwrites
the `%errorlevel%` variable, causing the batch script to exit with a success
code even when startup fails. To fix this, save the error level value in a
variable before calling `pause`, then use that saved value to exit the script
with the correct error code. Capture `%errorlevel%` into a temporary variable
immediately after detecting the failure condition in the if statement, then
after the `pause` command completes, exit with that saved error code using the
`exit /b` command with the saved variable reference.
---
Nitpick comments:
In `@examples/web_ui_demo/scripts/windows/Start-SpecR.ps1`:
- Around line 43-56: The script starts the API process with Start-Process but
immediately proceeds to display connection information and launch the web UI
without verifying the API is ready to accept requests. Add a health check wait
loop after the Start-Process line that launches the API node process. This loop
should poll the API endpoint at http://127.0.0.1:$ApiPort with a reasonable
timeout and retry interval to confirm the API is responding before allowing the
script to continue with the Write-Host statements and the Start-Process call
that opens the web UI demo.
In `@examples/web_ui_demo/Start-SpecR.sh`:
- Around line 30-38: The Start-SpecR.sh script starts the API process with `node
dist/index.js &` and assigns the PID to API_PID, but immediately launches the
demo server with `node server.mjs` without waiting for the API to be ready,
causing potential race conditions. After the API_PID assignment and before the
cd command that precedes launching the demo server, add a polling loop that
checks if the API is ready to accept requests (you can use curl or similar to
probe the API port specified by API_PORT) with a reasonable timeout and retry
count to ensure the API is listening before proceeding to start the demo server.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 98628c34-77fe-49a3-be5c-ff7cbd49d4f0
📒 Files selected for processing (17)
examples/web_ui_demo/.gitattributesexamples/web_ui_demo/README.mdexamples/web_ui_demo/Start-SpecR.batexamples/web_ui_demo/Start-SpecR.shexamples/web_ui_demo/css/app.cssexamples/web_ui_demo/index.htmlexamples/web_ui_demo/js/api.jsexamples/web_ui_demo/js/app.jsexamples/web_ui_demo/js/coordination.jsexamples/web_ui_demo/js/dropzone.jsexamples/web_ui_demo/js/labels.jsexamples/web_ui_demo/js/modal.jsexamples/web_ui_demo/js/popover.jsexamples/web_ui_demo/js/tree.jsexamples/web_ui_demo/js/web.jsexamples/web_ui_demo/scripts/windows/Start-SpecR.ps1examples/web_ui_demo/server.mjs
Five lint-independent correctness/robustness fixes surfaced by CodeRabbit review of PR #225. The size/console/stylelint findings are declined on the PR: examples/ is intentionally outside the eslint/prettier surface (pnpm lint targets src/ only) and the repo ships no stylelint. - web.js: when the same (from→to) citation recurs, the most severe status now wins the arc colour (STATUS_RANK) so a broken ref isn't masked by an earlier resolved one. - server.mjs: guard decodeURIComponent against malformed escapes (e.g. "%ZZ") and return 400 instead of throwing before the handler's try block. - modal.js: opening a second modal now runs the prior modal's finish(null) instead of bare-removing its veil, so its capture-phase keydown listener is detached and its promise resolves (was an orphaned-listener leak). - app.js: commitTextEdit resyncs the board to server truth on partial failure rather than retrying on top of half-applied mutations. - Start-SpecR.bat: capture and re-emit the powershell exit code so callers see startup failures (pause no longer clobbers %errorlevel%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses both CodeRabbit body nitpicks on PR #225. The launchers started the demo server immediately after backgrounding the API, racing boot and surfacing avoidable "API unavailable" failures on first run. Both now poll GET /health (up to ~30s) before continuing. The PowerShell wait sits inside the existing try block so the finally still stops the API process if readiness times out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Handled the two CodeRabbit body nitpicks (no review thread to resolve):
The 17 actionable inline threads are all replied-to and resolved: 5 genuine logic/robustness fixes in 4bc6176 (web.js edge-status precedence, server.mjs |
The launchers hard-coded API 3000 / web 3001 and failed when another dev server already held those ports (e.g. a local Next.js app on 3000). Both Start-SpecR.sh and Start-SpecR.ps1 now probe from the desired port and increment to the first free one, propagating the chosen API port to the bind (PORT), the demo proxy (SPECR_API_BASE), and the health wait, and the chosen web port to the demo server. The web port also skips the API port so they never collide. Port availability is tested by attempting to bind (Node net.createServer in bash, TcpListener in PowerShell) — the same operation the servers perform — which is more reliable than /dev/tcp (often compiled out of bash) or ss/lsof/nc (not always installed). SPECR_PORT / SPECR_WEB_PORT still set the starting point. Verified end-to-end with 3000 occupied: API bumped to 3001, demo to 3002, health wait passed, both endpoints served. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 21, 2026
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.
Summary
examples/web_ui_demo/so the PR no longer carries API/backend changes from the long-livedmockupbranch.examples/web_ui_demo/server.mjs) so the demo can call a local SpecR API without changingsrc/to serve demo assets or CORS.examples/web_ui_demo/README.md.Boundary Check
The final branch diff against
mainis limited toexamples/web_ui_demo/**. The explicit non-example audit returned no paths:git diff --name-only main..HEAD -- ':!examples/web_ui_demo/**'Validation
bash -n examples/web_ui_demo/Start-SpecR.shnode --check examples/web_ui_demo/server.mjsnode --check examples/web_ui_demo/js/*.js(run per file)pnpm buildNotes: PowerShell is not available in the local environment, so
examples/web_ui_demo/scripts/windows/Start-SpecR.ps1was not parse-checked locally. A localhost smoke test confirmed static files serve throughserver.mjs;/healthproxied to an unrelated process already running on local port 3000 in this environment, so live SpecR API compatibility was not asserted.Summary by CodeRabbit
.gitattributesto standardize line endings across Windows batch/PowerShell and Unix shell scripts.