dohertj2/jdescopingtool
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
6e2decd21f34ddb8c0d267d2c8342c8b490546e8
jdescopingtool/openspec/changes/questions.md
T
Joseph Doherty 26ff8d9b4f Initial commit: JDE Scoping Tool migration project 
Set up repository with legacy .NET Framework 4.8 source (OLD/),
new .NET 10 Blazor solution (NEW/), OpenSpec specifications,
documentation, and project configuration.
2026-01-02 07:43:29 -05:00

15 KiB
Raw Blame History

Unanswered Questions

This file collects questions that cannot be answered with best practices alone and require user decision.

Best Practice Decisions (Already Applied)

These common questions have been pre-answered with best practices:

Question Decision Rationale
Async vs sync methods Async-first with CancellationToken Modern .NET pattern, better scalability
Exception handling Custom typed exceptions per layer Clear error propagation, testability
Configuration IOptions pattern Type-safe, hot-reload support
Logging ILogger with structured logging Framework integration, filtering
Testing xUnit + Shouldly + NSubstitute Project constraints (no FluentAssertions)
DI lifetime Scoped for DB, Singleton for config Standard patterns
Nullable refs Enable project-wide Modern C# safety
Oracle driver Oracle.ManagedDataAccess.Core Single driver for JDE and CMS
Excel library ClosedXML (MIT) Free, replaces EPPlus
Date types DATETIME2(7) Better precision

Phase-Specific Questions

Questions below require user input before implementation.

Phase 2: setup-solution-foundation

Q2.1: Cron schedule disable pattern

  • Context: Design uses empty cron strings to disable schedules in Development
  • Options: (a) Empty string = disabled, (b) Add Enabled flag, (c) Use string? with null = disabled
  • Recommendation: Use Enabled flag for clarity
  • Impact: DataSyncOptions, schedule evaluation logic

Q2.2: Temp directory portability

  • Context: ExcelExportOptions defaults to /tmp/lotfinder which is Linux-only
  • Options: (a) Use Path.GetTempPath(), (b) Make required config, (c) Platform-detect at runtime
  • Recommendation: Path.Combine(Path.GetTempPath(), "lotfinder") as default
  • Impact: Excel export temp file handling

Phase 3: implement-domain-models

Q3.1: Requirement count discrepancy

  • Context: Tasks claim 52 requirements but base spec has 34 sections
  • Options: (a) Count includes delta spec additions, (b) Update to correct count
  • Recommendation: Update to accurate count from base spec
  • Impact: Acceptance criteria verification

Q3.2: Namespace convention

  • Context: Base spec references ScopingTool.Domain.Models, tasks use JdeScoping.Core/Models
  • Options: (a) Use JdeScoping.Core (matches solution), (b) Use ScopingTool.Domain (matches spec)
  • Recommendation: Use JdeScoping.Core.Models (matches existing solution structure)
  • Impact: All entity file locations, using statements

Phase 4: implement-data-access

Q4.1: Repository method counts

  • Context: Tasks list wrong method counts (17/18) vs spec (23/22)
  • Options: Update task counts to match spec
  • Recommendation: Fix counts during implementation
  • Impact: Task list accuracy

Phase 5: implement-data-sync

Q5.1: JDE/CMS fetcher deferral

  • Context: Proposal defers real JDE/CMS connectivity and CMS circuit breaker
  • Options: (a) Implement real fetchers now, (b) Use mock fetchers initially, (c) Implement core service with stub fetchers
  • Recommendation: Implement core service pattern with interface abstraction, defer real Oracle/Sybase connectivity
  • Impact: Testing approach, deployment timeline

Phase 6: implement-search-processing

Q6.1: Stored Procedure for Downstream Traversal

  • Context: Legacy system generates WHILE loop inline in T4 template for downstream work order traversal (up to 20 iterations). Base spec suggests stored procedure.
  • Options: (a) Create new stored procedure dbo.TraverseWorkOrders, (b) Keep inline WHILE loop in SqlKata-generated SQL, (c) Use C# iterative approach with multiple queries
  • Recommendation: Option (a) - stored procedure reduces network round trips and maintains transaction consistency
  • Impact: Requires adding stored procedure to Phase 1 (migrate-database-schema) if not already present

Q6.2: MIS Extraction Query Strategy

  • Context: MIS data extraction queries do NOT join #Temp_WO per Codex review findings. MIS extraction is somewhat independent of main search.
  • Options: (a) Keep MIS extraction as separate SqlKata queries, (b) Create dedicated stored procedure for MIS extraction, (c) Separate MIS extraction into its own service
  • Recommendation: Option (a) - separate SqlKata queries via MisQueryBuilder class
  • Impact: Affects ISearchQueryBuilder interface design

Q6.3: Filter Entry Location

  • Context: Legacy filter entry classes are in OLD/WorkerService/Models/Reporting/. Design places them in JdeScoping.SearchProcessing/Models/FilterEntries/.
  • Options: (a) Place in SearchProcessing project, (b) Place in Domain Models project, (c) Create separate JdeScoping.Shared project
  • Recommendation: Option (a) - filter entries are specific to search processing
  • Impact: Project references and potential code duplication

Q6.4: Streaming vs Materialization Default

  • Context: Design supports both IAsyncEnumerable<SearchResult> streaming and Task<SearchModel> materialization.
  • Options: (a) Primary API returns streaming, caller materializes, (b) Primary API returns materialized, streaming is secondary, (c) Two equal methods, caller chooses
  • Recommendation: Option (c) - provide both ExecuteSearchAsync (streaming) and ExecuteSearchToModelAsync (materialized)
  • Impact: Excel export needs materialized; real-time progress benefits from streaming

Phase 7: implement-excel-export

Q7.1: Test Data Source

  • Context: Integration tests need sample search results to generate Excel files
  • Options: (a) Create mock data factories in test project, (b) Use shared test data from Phase 3, (c) Load sample data from JSON fixtures
  • Recommendation: Option (a) - mock data factories reusable across test projects
  • Impact: Test project structure and data sharing

Q7.2: Debug File Output Location

  • Context: Legacy code wrote debug Excel files to disk for troubleshooting. Design includes DebugWriteToFile option.
  • Options: (a) Keep debug file feature but default to disabled, (b) Remove debug file feature, (c) Replace with structured logging
  • Recommendation: Option (a) - keep for backward compatibility, disabled by default
  • Impact: Configuration schema and troubleshooting capabilities

Q7.3: Empty Results Handling

  • Context: What should happen when SearchResults is empty (not null)?
  • Options: (a) Generate sheet with headers only, (b) Generate sheet with "No results found" message, (c) Skip results sheet entirely
  • Recommendation: Option (a) - generate sheet with headers only (matches legacy behavior)
  • Impact: Sheet generation logic and user expectations

Q7.4: Large Export Memory Threshold

  • Context: Design notes large exports (>100K rows) may need streaming approach
  • Options: (a) Implement now with configurable threshold, (b) Defer to future phase if memory issues arise, (c) Add memory monitoring without streaming
  • Recommendation: Option (b) - defer to future phase; current in-memory approach handles typical workloads
  • Impact: Scope of Phase 7 and future optimization work

Codex MCP Review Findings (Phase 6-7)

Phase 6 Codex Findings

F6.1: Work order traversal design conflict (HIGH)

  • Finding: Base spec fixes 20 iterations with seed-list API, but change spec makes it configurable (MaxTraversalIterations) with TraverseDownstreamAsync
  • Action: Choose authoritative model and update spec/tasks
  • Recommendation: Keep configurable approach (more flexible), update base spec to align

F6.2: SqlKata parameter naming (HIGH)

  • Finding: Delta spec requires @p_* named parameters and singleton compiler, but design uses new SqlServerCompiler() which defaults to @p0 style
  • Action: Add parameter naming requirements to tasks, use singleton compiler pattern

F6.3: MIS behavior requirements missing from tasks (MEDIUM)

  • Finding: MIS rules (always include non-match results when ExtractMisData=true, min-only/max-only timespan handling) not explicit in tasks
  • Action: Add explicit task items for MIS behavior requirements

F6.4: Filter handler priority order (LOW)

  • Finding: Tasks don't require setting/validating priority order per spec delta
  • Action: Add explicit priority ordering requirements to filter handler tasks

Phase 7 Codex Findings

F7.1: Sheet protection inconsistency (HIGH)

  • Finding: Tasks call for protecting data sheets, spec says Search Results is unprotected, missing allowlist + unlocked 1000x1000 extension area
  • Action: Reconcile protection behavior between spec/design/tasks
  • Recommendation: Follow spec (Search Results unprotected), add allowlist + unlock range for other sheets

F7.2: ShowHeader handling missing (HIGH)

  • Finding: Attribute-driven tables omit ShowHeader handling for merged section header rows (needed for criteria filter tables)
  • Action: Add ShowHeader support to AttributeTableWriter

F7.3: Project dependency name (MEDIUM)

  • Finding: Tasks reference JdeScoping.Domain but NEW/src only has JdeScoping.Core
  • Action: Update dependency target to correct project name

F7.4: Multi-sheet rules incomplete (MEDIUM)

  • Finding: No explicit requirements for sheet order, totals row disabled, empty-but-not-null MIS results
  • Action: Add explicit tasks/tests for multi-sheet behavior

Phase 8: implement-web-api

Q8.1: LDAP Connection Pooling

  • Context: Current design creates and disposes LdapConnection for each authentication attempt
  • Options: (a) Keep simple connection-per-request, (b) Implement connection pooling
  • Recommendation: Option (a) - connection-per-request for simplicity, authentication is infrequent
  • Impact: Performance for high-volume login scenarios

Q8.2: SignalR Authentication Requirement

  • Context: Legacy StatusHub does not require authentication for connections
  • Options: (a) Keep hub open to all connections, (b) Require authentication for hub connections
  • Recommendation: Option (a) - keep open, status updates are not user-specific and user base is internal
  • Impact: Security posture for SignalR connections

Q8.3: Admin Bypass Configuration

  • Context: Legacy code has hardcoded username bypass for group check ("dohertj2")
  • Options: (a) Remove entirely, (b) Make configurable via AuthOptions.AdminBypassUsers array
  • Recommendation: Option (b) - make configurable for dev flexibility without hardcoding
  • Impact: Developer experience during testing

Q8.4: File Upload Size Limits

  • Context: Legacy has no explicit file size limits for Excel uploads
  • Options: (a) No limits, (b) Configure reasonable limit (e.g., 10MB)
  • Recommendation: Option (b) - configure 10MB limit to prevent abuse
  • Impact: User experience for bulk data uploads

Q8.5: CORS Configuration for Blazor WASM

  • Context: If Blazor client is served from a different origin than API
  • Options: (a) Same-origin only, (b) Configure CORS for Blazor origin
  • Recommendation: Depends on deployment - same-origin if single host, CORS with explicit origins if separate
  • Impact: Blazor WASM deployment architecture

Phase 9: implement-blazor-ui

Q9.1: Mock Services Strategy

  • Context: Phase 8 (API) is not yet implemented. UI needs data to function.
  • Options: (a) Create mock services that return static/test data, (b) Create mock API endpoints in JdeScoping.Host, (c) Wait for Phase 8 before implementing UI
  • Recommendation: Option (a) - mock services allow parallel development
  • Impact: API client services can use mock implementations initially

Q9.2: Test Strategy for Blazor Components

  • Context: bUnit is standard for Blazor component testing but adds complexity
  • Options: (a) Add bUnit tests as part of this phase, (b) Defer component testing to separate phase, (c) Manual testing only
  • Recommendation: Option (b) - focus on functional implementation first
  • Impact: No test tasks included in current tasks.md

Q9.3: Navigation Menu Visibility

  • Context: Base spec shows minimal header with just title and user menu
  • Options: (a) Add navigation links to header, (b) Keep minimal header as specified, (c) Add collapsible sidebar navigation
  • Recommendation: Option (a) - header navigation for quick access
  • Impact: MainLayout.razor design

Codex MCP Review Findings (Phase 8-9)

Phase 8 Codex Findings

F8.1: NEW/src diverges from spec/plan (HIGH)

  • Finding: Current auth API shape and LDAP behavior don't match spec (no GetUserInfoAsync, no multi-server/group checks), /hubs/status not mapped
  • Action: Align implementation plan with actual repo layout, add hub mapping

F8.2: SPA behaviors not in tasks (HIGH)

  • Finding: CORS with credentials, JSON error responses, SignalR reconnection support are required but not planned
  • Action: Add explicit tasks for CORS, API error JSON behavior, SignalR reconnection

F8.3: LDAP scenarios incomplete (MEDIUM)

  • Finding: Legacy has hardcoded group-bypass user, design adds AdminBypassUsers but doesn't use it, GetUserInfoAsync throws NotSupported
  • Action: Decide on admin bypass path, clarify GetUserInfoAsync behavior

F8.4: Project structure issues (MEDIUM)

  • Finding: Tasks reference JdeScoping.Domain but only JdeScoping.Core exists; SearchController scheduled before StatusHub
  • Action: Align to actual project structure (use JdeScoping.Core), reorder dependencies

F8.5: SearchUpdate missing fields (LOW)

  • Finding: Design omits SubmitDT/StartDT/EndDT fields that exist in spec/legacy model
  • Action: Add missing timestamp fields to SearchUpdate DTO

Phase 9 Codex Findings

F9.1: Route conflict (HIGH)

  • Finding: Home.razor owns /, but tasks add / to Searches.razor without removal/rename
  • Action: Remove template pages (Home.razor, Counter.razor, Weather.razor) before adding Searches

F9.2: JWT auth requirements incomplete (HIGH)

  • Finding: Token attachment and 401 re-auth redirect required but not captured in auth tasks
  • Action: Add tasks for auth header injection and 401 redirect handling

F9.3: SearchStatus enum mismatch (HIGH)

  • Finding: Spec uses New/Submitted/Started/Ended/Error but JdeScoping.Core uses Queued/Processing/Completed/Failed
  • Action: Decide on enum alignment - either map or standardize

F9.4: SearchEdit behaviors missing (HIGH)

  • Finding: Search-type detection, submit confirmation, download results, Extract MIS checkbox required but not in tasks
  • Action: Expand SearchEdit tasks with spec behaviors

F9.5: SignalR reconnection requirements partial (MEDIUM)

  • Finding: Backoff schedule + UI connection state + reconnect logging required but tasks only say "auto-reconnect"
  • Action: Add explicit tasks for reconnection UI and logging

F9.6: Clear Data confirmation missing (MEDIUM)

  • Finding: Clear Data confirmations required per spec but filter panel tasks say "Clear empties grid" with no confirmation
  • Action: Add confirmation dialog to Clear Data functionality

F9.7: Template pages cleanup (LOW)

  • Finding: Blazor template pages (Home, Counter, Weather, NavMenu) should be removed
  • Action: Add task to remove template pages as part of UI change set

F9.8: NuGet version mismatch (LOW)

  • Finding: Base spec calls for Radzen 5./SignalR 9. but design uses Radzen 8.4.2/SignalR 10.0.1
  • Action: Update spec to reflect actual .NET 10 versions
Reference in New Issue View Git Blame Copy Permalink